Skip to content

Commit cd71b17

Browse files
llvm-beanzV-FEXrtalsepkow
authored
[Validation] Drill through chained GEPs for TGSM (#8575)
DXIL requires that TGSM pointers originate from a global variable in address space 3, and that any use of a pointer resolve to one and only one possible global variable. In SM 6.9 with the introduction of native vectors to DXIL it is now possible to end up with a chain of GEP and/or bitcast instructions in the access chain for a TGSM pointer. To address this the validator needs to be able to walk up a TGSM chain to find the one unambiguous global. Fixes #8571 Assisted by Claude Opus 4.7 --------- Co-authored-by: Ashley Coleman <ascoleman@microsoft.com> Co-authored-by: Alex Sepkowski <alexsepkowski@gmail.com>
1 parent dd3152d commit cd71b17

3 files changed

Lines changed: 176 additions & 14 deletions

File tree

‎lib/DxilValidation/DxilValidation.cpp‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3305,21 +3305,24 @@ static void ValidateFunctionBody(Function *F, ValidationContext &ValCtx) {
33053305

33063306
if (PointerType *PT = dyn_cast<PointerType>(I.getType())) {
33073307
if (PT->getAddressSpace() == DXIL::kTGSMAddrSpace) {
3308-
if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(&I)) {
3309-
Value *Ptr = GEP->getPointerOperand();
3310-
// Allow inner constant GEP
3311-
if (isa<ConstantExpr>(Ptr) && isa<GEPOperator>(Ptr))
3312-
Ptr = cast<GEPOperator>(Ptr)->getPointerOperand();
3313-
if (!isa<GlobalVariable>(Ptr)) {
3314-
ValCtx.EmitInstrError(
3315-
&I, ValidationRule::InstrFailToResloveTGSMPointer);
3308+
// Walk through GEPs and bitcasts to ensure the pointer ultimately
3309+
// comes from a global variable. This was unnecessary before SM 6.9
3310+
// because everything was scalarized, but now we can have arrays of
3311+
// vectors in TGSM, so we need to allow GEPs and bitcasts.
3312+
if (isa<GetElementPtrInst>(&I) || isa<BitCastInst>(&I)) {
3313+
Value *Ptr = cast<Instruction>(&I)->getOperand(0);
3314+
while (Ptr) {
3315+
if (GEPOperator *GEP = dyn_cast<GEPOperator>(Ptr)) {
3316+
Ptr = GEP->getPointerOperand();
3317+
continue;
3318+
}
3319+
if (BitCastOperator *BC = dyn_cast<BitCastOperator>(Ptr)) {
3320+
Ptr = BC->getOperand(0);
3321+
continue;
3322+
}
3323+
break;
33163324
}
3317-
} else if (BitCastInst *BCI = dyn_cast<BitCastInst>(&I)) {
3318-
Value *Ptr = BCI->getOperand(0);
3319-
// Allow inner constant GEP
3320-
if (isa<ConstantExpr>(Ptr) && isa<GEPOperator>(Ptr))
3321-
Ptr = cast<GEPOperator>(Ptr)->getPointerOperand();
3322-
if (!isa<GetElementPtrInst>(Ptr) && !isa<GlobalVariable>(Ptr)) {
3325+
if (!isa<GlobalVariable>(Ptr)) {
33233326
ValCtx.EmitInstrError(
33243327
&I, ValidationRule::InstrFailToResloveTGSMPointer);
33253328
}

‎tools/clang/test/LitDXILValidation/GroupShared/tgsm-chained-gep-ambiguous.ll‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
; RUN: not %dxv %s 2>&1 | FileCheck %s
2+
3+
; Negative tests for the TGSM pointer chain walk added with chained GEP /
4+
; bitcast support. The validator only walks through GEP and bitcast
5+
; operators / instructions; any other construct (phi, select, etc.) in
6+
; the middle of a chain leaves the ultimate base ambiguous and must be
7+
; rejected. Both the phi/select itself (which produces a TGSM pointer
8+
; that is not a GEP or bitcast) and any GEP / bitcast that consumes it
9+
; must be reported.
10+
11+
target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
12+
target triple = "dxil-ms-dx"
13+
14+
@"\01?GA@@3PAIA" = external addrspace(3) global [32 x i32], align 4
15+
@"\01?GB@@3PAIA" = external addrspace(3) global [32 x i32], align 4
16+
17+
define void @main() {
18+
entry:
19+
%tid = call i32 @dx.op.threadIdInGroup.i32(i32 95, i32 0)
20+
%cond = icmp ult i32 %tid, 16
21+
%ga = getelementptr [32 x i32], [32 x i32] addrspace(3)* @"\01?GA@@3PAIA", i32 0, i32 %tid
22+
%gb = getelementptr [32 x i32], [32 x i32] addrspace(3)* @"\01?GB@@3PAIA", i32 0, i32 %tid
23+
24+
; --- select between two TGSM GEPs, then chain a GEP through the select ---
25+
; The select itself directly produces a TGSM pointer that is neither a
26+
; GEP nor a bitcast, so it must be rejected.
27+
; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable.
28+
; CHECK-NEXT: note: at '{{.*}}select{{.*}}' in block 'entry' of function 'main'.
29+
%sel = select i1 %cond, i32 addrspace(3)* %ga, i32 addrspace(3)* %gb
30+
; A GEP that walks through the select must also be rejected: the chain
31+
; walker stops at the select (not a GEP / bitcast / GlobalVariable).
32+
; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable.
33+
; CHECK-NEXT: note: at '%sel_gep = getelementptr i32, i32 addrspace(3)* %sel, i32 0' in block 'entry' of function 'main'.
34+
%sel_gep = getelementptr i32, i32 addrspace(3)* %sel, i32 0
35+
store i32 1, i32 addrspace(3)* %sel_gep, align 4
36+
37+
; A bitcast that walks through the select must likewise be rejected.
38+
; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable.
39+
; CHECK-NEXT: note: at '%sel_bc = bitcast i32 addrspace(3)* %sel to float addrspace(3)*' in block 'entry' of function 'main'.
40+
%sel_bc = bitcast i32 addrspace(3)* %sel to float addrspace(3)*
41+
store float 2.000000e+00, float addrspace(3)* %sel_bc, align 4
42+
43+
br i1 %cond, label %then, label %else
44+
45+
then:
46+
%ga2 = getelementptr [32 x i32], [32 x i32] addrspace(3)* @"\01?GA@@3PAIA", i32 0, i32 %tid
47+
br label %merge
48+
49+
else:
50+
%gb2 = getelementptr [32 x i32], [32 x i32] addrspace(3)* @"\01?GB@@3PAIA", i32 0, i32 %tid
51+
br label %merge
52+
53+
merge:
54+
; --- phi joining two TGSM GEPs, then chain a GEP/bitcast through the phi ---
55+
; The phi itself produces an ambiguous TGSM pointer.
56+
; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable.
57+
; CHECK-NEXT: note: at '{{.*}}phi{{.*}}' in block 'merge' of function 'main'.
58+
%p = phi i32 addrspace(3)* [ %ga2, %then ], [ %gb2, %else ]
59+
; A GEP through the phi must be rejected: the chain walk reaches the
60+
; phi which is neither a GEP, a bitcast, nor a GlobalVariable.
61+
; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable.
62+
; CHECK-NEXT: note: at '%phi_gep = getelementptr i32, i32 addrspace(3)* %p, i32 0' in block 'merge' of function 'main'.
63+
%phi_gep = getelementptr i32, i32 addrspace(3)* %p, i32 0
64+
store i32 3, i32 addrspace(3)* %phi_gep, align 4
65+
66+
; Likewise a bitcast through the phi must be rejected, even when the
67+
; chain has multiple GEP / bitcast hops before the phi appears.
68+
; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable.
69+
; CHECK-NEXT: note: at '%phi_bc = bitcast i32 addrspace(3)* %phi_gep to float addrspace(3)*' in block 'merge' of function 'main'.
70+
%phi_bc = bitcast i32 addrspace(3)* %phi_gep to float addrspace(3)*
71+
store float 4.000000e+00, float addrspace(3)* %phi_bc, align 4
72+
73+
; CHECK: Validation failed.
74+
ret void
75+
}
76+
77+
declare i32 @dx.op.threadIdInGroup.i32(i32, i32) #0
78+
79+
attributes #0 = { nounwind readnone }
80+
81+
!llvm.ident = !{!0}
82+
!dx.version = !{!1}
83+
!dx.valver = !{!2}
84+
!dx.shaderModel = !{!3}
85+
!dx.entryPoints = !{!4}
86+
87+
!0 = !{!"dxc test"}
88+
!1 = !{i32 1, i32 9}
89+
!2 = !{i32 1, i32 10}
90+
!3 = !{!"cs", i32 6, i32 9}
91+
!4 = !{void ()* @main, !"main", null, null, !5}
92+
!5 = !{i32 4, !6}
93+
!6 = !{i32 64, i32 1, i32 1}

‎tools/clang/test/LitDXILValidation/GroupShared/tgsm-chained-gep.ll‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
; RUN: %dxv %s 2>&1 | FileCheck %s
2+
3+
; Regression test for TGSM pointer validation when SM 6.9 preserves native
4+
; vectors. The DXIL validator must accept TGSM pointers produced by chained
5+
; getelementptr and/or bitcast instructions whose ultimate base is an
6+
; unambiguous groupshared global variable. Prior to this fix, the validator
7+
; only allowed a single GEP instruction (optionally over a constant-expression
8+
; GEP), and a single bitcast whose operand was a single GEP; arbitrary chains
9+
; of GEP and bitcast instructions were incorrectly reported as "TGSM pointers
10+
; must originate from an unambiguous TGSM global variable" even though they
11+
; were unambiguously rooted at a TGSM global.
12+
13+
; CHECK: Validation succeeded.
14+
15+
target datalayout = "e-m:e-p:32:32-i1:32-i8:32-i16:32-i32:32-i64:64-f16:32-f32:32-f64:64-n8:16:32:64"
16+
target triple = "dxil-ms-dx"
17+
18+
@"\01?GS@@3PAV?$matrix@M$02$02@@A.v" = external addrspace(3) global [64 x <9 x float>], align 4
19+
20+
define void @main() {
21+
%tid = call i32 @dx.op.threadIdInGroup.i32(i32 95, i32 0)
22+
; First-level GEP: indexes the per-thread element of the groupshared
23+
; matrix array. Its base is the global, so it is unambiguous.
24+
%elt = getelementptr [64 x <9 x float>], [64 x <9 x float>] addrspace(3)* @"\01?GS@@3PAV?$matrix@M$02$02@@A.v", i32 0, i32 %tid
25+
store <9 x float> zeroinitializer, <9 x float> addrspace(3)* %elt, align 4
26+
; Second-level GEP: indexes into the per-thread vector. Its base is a
27+
; GEP *instruction* (not a constant expression), which previously
28+
; failed validation but is still unambiguously rooted at the global.
29+
%scalar = getelementptr <9 x float>, <9 x float> addrspace(3)* %elt, i32 0, i32 0
30+
store float 1.000000e+00, float addrspace(3)* %scalar, align 4
31+
32+
; Bitcast of a GEP instruction. Chain: BC -> GEP -> Global.
33+
%bc1 = bitcast <9 x float> addrspace(3)* %elt to <9 x i32> addrspace(3)*
34+
store <9 x i32> zeroinitializer, <9 x i32> addrspace(3)* %bc1, align 4
35+
36+
; GEP through a bitcast through a GEP. Chain: GEP -> BC -> GEP -> Global.
37+
%bc1_scalar = getelementptr <9 x i32>, <9 x i32> addrspace(3)* %bc1, i32 0, i32 3
38+
store i32 7, i32 addrspace(3)* %bc1_scalar, align 4
39+
40+
; Bitcast of a bitcast. Chain: BC -> BC -> GEP -> Global.
41+
%bc2 = bitcast <9 x i32> addrspace(3)* %bc1 to <9 x float> addrspace(3)*
42+
store <9 x float> zeroinitializer, <9 x float> addrspace(3)* %bc2, align 4
43+
44+
; Bitcast of a constant-expression GEP. Chain: BC -> ConstGEP -> Global.
45+
%bcconst = bitcast <9 x float> addrspace(3)* getelementptr inbounds ([64 x <9 x float>], [64 x <9 x float>] addrspace(3)* @"\01?GS@@3PAV?$matrix@M$02$02@@A.v", i32 0, i32 0) to <9 x i32> addrspace(3)*
46+
store <9 x i32> zeroinitializer, <9 x i32> addrspace(3)* %bcconst, align 4
47+
ret void
48+
}
49+
50+
declare i32 @dx.op.threadIdInGroup.i32(i32, i32) #0
51+
52+
attributes #0 = { nounwind readnone }
53+
54+
!llvm.ident = !{!0}
55+
!dx.version = !{!1}
56+
!dx.valver = !{!2}
57+
!dx.shaderModel = !{!3}
58+
!dx.entryPoints = !{!4}
59+
60+
!0 = !{!"dxc test"}
61+
!1 = !{i32 1, i32 9}
62+
!2 = !{i32 1, i32 10}
63+
!3 = !{!"cs", i32 6, i32 9}
64+
!4 = !{void ()* @main, !"main", null, null, !5}
65+
!5 = !{i32 4, !6}
66+
!6 = !{i32 64, i32 1, i32 1}

0 commit comments

Comments
 (0)