-
Notifications
You must be signed in to change notification settings - Fork 880
[Validation] Drill through chained GEPs for TGSM #8575
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
llvm-beanz
wants to merge
5
commits into
microsoft:main
Choose a base branch
from
llvm-beanz:8571
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+176
−14
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
7f0dad0
[Validation] Drill through chained GEPs for TGSM
llvm-beanz 5640057
Update lib/DxilValidation/DxilValidation.cpp
llvm-beanz a18615e
clang-format
llvm-beanz 23e2dc2
Roll back to original code
llvm-beanz eee321f
Update lib/DxilValidation/DxilValidation.cpp
llvm-beanz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
93 changes: 93 additions & 0 deletions
93
tools/clang/test/LitDXILValidation/GroupShared/tgsm-chained-gep-ambiguous.ll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| ; RUN: not %dxv %s 2>&1 | FileCheck %s | ||
|
|
||
| ; Negative tests for the TGSM pointer chain walk added with chained GEP / | ||
| ; bitcast support. The validator only walks through GEP and bitcast | ||
| ; operators / instructions; any other construct (phi, select, etc.) in | ||
| ; the middle of a chain leaves the ultimate base ambiguous and must be | ||
| ; rejected. Both the phi/select itself (which produces a TGSM pointer | ||
| ; that is not a GEP or bitcast) and any GEP / bitcast that consumes it | ||
| ; must be reported. | ||
|
|
||
| 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" | ||
| target triple = "dxil-ms-dx" | ||
|
|
||
| @"\01?GA@@3PAIA" = external addrspace(3) global [32 x i32], align 4 | ||
| @"\01?GB@@3PAIA" = external addrspace(3) global [32 x i32], align 4 | ||
|
|
||
| define void @main() { | ||
| entry: | ||
| %tid = call i32 @dx.op.threadIdInGroup.i32(i32 95, i32 0) | ||
| %cond = icmp ult i32 %tid, 16 | ||
| %ga = getelementptr [32 x i32], [32 x i32] addrspace(3)* @"\01?GA@@3PAIA", i32 0, i32 %tid | ||
| %gb = getelementptr [32 x i32], [32 x i32] addrspace(3)* @"\01?GB@@3PAIA", i32 0, i32 %tid | ||
|
|
||
| ; --- select between two TGSM GEPs, then chain a GEP through the select --- | ||
| ; The select itself directly produces a TGSM pointer that is neither a | ||
| ; GEP nor a bitcast, so it must be rejected. | ||
| ; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable. | ||
| ; CHECK-NEXT: note: at '{{.*}}select{{.*}}' in block 'entry' of function 'main'. | ||
| %sel = select i1 %cond, i32 addrspace(3)* %ga, i32 addrspace(3)* %gb | ||
| ; A GEP that walks through the select must also be rejected: the chain | ||
| ; walker stops at the select (not a GEP / bitcast / GlobalVariable). | ||
| ; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable. | ||
| ; CHECK-NEXT: note: at '%sel_gep = getelementptr i32, i32 addrspace(3)* %sel, i32 0' in block 'entry' of function 'main'. | ||
| %sel_gep = getelementptr i32, i32 addrspace(3)* %sel, i32 0 | ||
| store i32 1, i32 addrspace(3)* %sel_gep, align 4 | ||
|
|
||
| ; A bitcast that walks through the select must likewise be rejected. | ||
| ; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable. | ||
| ; CHECK-NEXT: note: at '%sel_bc = bitcast i32 addrspace(3)* %sel to float addrspace(3)*' in block 'entry' of function 'main'. | ||
| %sel_bc = bitcast i32 addrspace(3)* %sel to float addrspace(3)* | ||
| store float 2.000000e+00, float addrspace(3)* %sel_bc, align 4 | ||
|
|
||
| br i1 %cond, label %then, label %else | ||
|
|
||
| then: | ||
| %ga2 = getelementptr [32 x i32], [32 x i32] addrspace(3)* @"\01?GA@@3PAIA", i32 0, i32 %tid | ||
| br label %merge | ||
|
|
||
| else: | ||
| %gb2 = getelementptr [32 x i32], [32 x i32] addrspace(3)* @"\01?GB@@3PAIA", i32 0, i32 %tid | ||
| br label %merge | ||
|
|
||
| merge: | ||
| ; --- phi joining two TGSM GEPs, then chain a GEP/bitcast through the phi --- | ||
| ; The phi itself produces an ambiguous TGSM pointer. | ||
| ; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable. | ||
| ; CHECK-NEXT: note: at '{{.*}}phi{{.*}}' in block 'merge' of function 'main'. | ||
| %p = phi i32 addrspace(3)* [ %ga2, %then ], [ %gb2, %else ] | ||
| ; A GEP through the phi must be rejected: the chain walk reaches the | ||
| ; phi which is neither a GEP, a bitcast, nor a GlobalVariable. | ||
| ; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable. | ||
| ; CHECK-NEXT: note: at '%phi_gep = getelementptr i32, i32 addrspace(3)* %p, i32 0' in block 'merge' of function 'main'. | ||
| %phi_gep = getelementptr i32, i32 addrspace(3)* %p, i32 0 | ||
| store i32 3, i32 addrspace(3)* %phi_gep, align 4 | ||
|
|
||
| ; Likewise a bitcast through the phi must be rejected, even when the | ||
| ; chain has multiple GEP / bitcast hops before the phi appears. | ||
| ; CHECK: error: TGSM pointers must originate from an unambiguous TGSM global variable. | ||
| ; CHECK-NEXT: note: at '%phi_bc = bitcast i32 addrspace(3)* %phi_gep to float addrspace(3)*' in block 'merge' of function 'main'. | ||
| %phi_bc = bitcast i32 addrspace(3)* %phi_gep to float addrspace(3)* | ||
| store float 4.000000e+00, float addrspace(3)* %phi_bc, align 4 | ||
|
|
||
| ; CHECK: Validation failed. | ||
| ret void | ||
| } | ||
|
|
||
| declare i32 @dx.op.threadIdInGroup.i32(i32, i32) #0 | ||
|
|
||
| attributes #0 = { nounwind readnone } | ||
|
|
||
| !llvm.ident = !{!0} | ||
| !dx.version = !{!1} | ||
| !dx.valver = !{!2} | ||
| !dx.shaderModel = !{!3} | ||
| !dx.entryPoints = !{!4} | ||
|
|
||
| !0 = !{!"dxc test"} | ||
| !1 = !{i32 1, i32 9} | ||
| !2 = !{i32 1, i32 10} | ||
| !3 = !{!"cs", i32 6, i32 9} | ||
| !4 = !{void ()* @main, !"main", null, null, !5} | ||
| !5 = !{i32 4, !6} | ||
| !6 = !{i32 64, i32 1, i32 1} | ||
66 changes: 66 additions & 0 deletions
66
tools/clang/test/LitDXILValidation/GroupShared/tgsm-chained-gep.ll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| ; RUN: %dxv %s 2>&1 | FileCheck %s | ||
|
|
||
| ; Regression test for TGSM pointer validation when SM 6.9 preserves native | ||
| ; vectors. The DXIL validator must accept TGSM pointers produced by chained | ||
| ; getelementptr and/or bitcast instructions whose ultimate base is an | ||
| ; unambiguous groupshared global variable. Prior to this fix, the validator | ||
| ; only allowed a single GEP instruction (optionally over a constant-expression | ||
| ; GEP), and a single bitcast whose operand was a single GEP; arbitrary chains | ||
| ; of GEP and bitcast instructions were incorrectly reported as "TGSM pointers | ||
| ; must originate from an unambiguous TGSM global variable" even though they | ||
| ; were unambiguously rooted at a TGSM global. | ||
|
|
||
| ; CHECK: Validation succeeded. | ||
|
|
||
| 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" | ||
| target triple = "dxil-ms-dx" | ||
|
|
||
| @"\01?GS@@3PAV?$matrix@M$02$02@@A.v" = external addrspace(3) global [64 x <9 x float>], align 4 | ||
|
|
||
| define void @main() { | ||
| %tid = call i32 @dx.op.threadIdInGroup.i32(i32 95, i32 0) | ||
| ; First-level GEP: indexes the per-thread element of the groupshared | ||
| ; matrix array. Its base is the global, so it is unambiguous. | ||
| %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 | ||
| store <9 x float> zeroinitializer, <9 x float> addrspace(3)* %elt, align 4 | ||
| ; Second-level GEP: indexes into the per-thread vector. Its base is a | ||
| ; GEP *instruction* (not a constant expression), which previously | ||
| ; failed validation but is still unambiguously rooted at the global. | ||
| %scalar = getelementptr <9 x float>, <9 x float> addrspace(3)* %elt, i32 0, i32 0 | ||
| store float 1.000000e+00, float addrspace(3)* %scalar, align 4 | ||
|
|
||
| ; Bitcast of a GEP instruction. Chain: BC -> GEP -> Global. | ||
| %bc1 = bitcast <9 x float> addrspace(3)* %elt to <9 x i32> addrspace(3)* | ||
| store <9 x i32> zeroinitializer, <9 x i32> addrspace(3)* %bc1, align 4 | ||
|
|
||
| ; GEP through a bitcast through a GEP. Chain: GEP -> BC -> GEP -> Global. | ||
| %bc1_scalar = getelementptr <9 x i32>, <9 x i32> addrspace(3)* %bc1, i32 0, i32 3 | ||
| store i32 7, i32 addrspace(3)* %bc1_scalar, align 4 | ||
|
|
||
| ; Bitcast of a bitcast. Chain: BC -> BC -> GEP -> Global. | ||
| %bc2 = bitcast <9 x i32> addrspace(3)* %bc1 to <9 x float> addrspace(3)* | ||
| store <9 x float> zeroinitializer, <9 x float> addrspace(3)* %bc2, align 4 | ||
|
|
||
| ; Bitcast of a constant-expression GEP. Chain: BC -> ConstGEP -> Global. | ||
| %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)* | ||
| store <9 x i32> zeroinitializer, <9 x i32> addrspace(3)* %bcconst, align 4 | ||
| ret void | ||
| } | ||
|
|
||
| declare i32 @dx.op.threadIdInGroup.i32(i32, i32) #0 | ||
|
|
||
| attributes #0 = { nounwind readnone } | ||
|
|
||
| !llvm.ident = !{!0} | ||
| !dx.version = !{!1} | ||
| !dx.valver = !{!2} | ||
| !dx.shaderModel = !{!3} | ||
| !dx.entryPoints = !{!4} | ||
|
|
||
| !0 = !{!"dxc test"} | ||
| !1 = !{i32 1, i32 9} | ||
| !2 = !{i32 1, i32 10} | ||
| !3 = !{!"cs", i32 6, i32 9} | ||
| !4 = !{void ()* @main, !"main", null, null, !5} | ||
| !5 = !{i32 4, !6} | ||
| !6 = !{i32 64, i32 1, i32 1} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the phi or select is choosing between multiple GEPs or bitcasts that all point to the same global variable, just at different offsets? Would this still be valid DXIL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be... but to be honest I can't even contrive how it would be possible to generate it from HLSL. Generating PHIs and selects on offsets is easy enough because you can just mess with the index, but the base address of an array isn't something HLSL allows you to conditionalize on.