-
Notifications
You must be signed in to change notification settings - Fork 14.2k
[DirectX] Implement Shader Flag Analysis for UAVsAtEveryStage
#137085
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
8342b32
Implement UAVsAtEveryStage shader flag
Icohedron 1f11675
Change vertex shader test function name and remove hlsl.export from it
Icohedron 5955a67
Simplify the logic for setting UAVsAtEveryStage
Icohedron f6410d0
Remove repeated comments among tests
Icohedron bd1be2a
Remove checks for no other shader flag is being set
Icohedron a956f03
Merge remote-tracking branch 'upstream/main' into shader-flag-UAVsAtE…
Icohedron 88793b6
Add additional fixes to unrelated shader-flag tests to account for UA…
Icohedron 362c442
Apply clang-format
Icohedron 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
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
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
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
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
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
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
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
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
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
35 changes: 35 additions & 0 deletions
35
llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-lib-valver1.7.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,35 @@ | ||
; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s | ||
; RUN: llc %s --filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=DXC | ||
|
||
; This test ensures that a library shader with a UAV gets the module and | ||
; shader feature flag UAVsAtEveryStage when the DXIL validator version is < 1.8 | ||
|
||
target triple = "dxil-pc-shadermodel6.5-library" | ||
|
||
; CHECK: Combined Shader Flags for Module | ||
; CHECK-NEXT: Shader Flags Value: 0x00010000 | ||
|
||
; CHECK: Note: shader requires additional functionality: | ||
; CHECK: UAVs at every shader stage | ||
|
||
; CHECK: Function test : 0x00000000 | ||
define void @test() "hlsl.export" { | ||
; RWBuffer<float> Buf : register(u0, space0) | ||
%buf0 = call target("dx.TypedBuffer", float, 1, 0, 1) | ||
@llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f32_1_0t( | ||
i32 0, i32 0, i32 1, i32 0, i1 false) | ||
ret void | ||
} | ||
|
||
!dx.valver = !{!1} | ||
!1 = !{i32 1, i32 7} | ||
|
||
!llvm.module.flags = !{!0} | ||
!0 = !{i32 1, !"dx.resmayalias", i32 1} | ||
|
||
; DXC: - Name: SFI0 | ||
; DXC-NEXT: Size: 8 | ||
; DXC-NEXT: Flags: | ||
; DXC: UAVsAtEveryStage: true | ||
; DXC: NextUnusedBit: false | ||
; DXC: ... |
27 changes: 27 additions & 0 deletions
27
llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-lib-valver1.8.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,27 @@ | ||
; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s | ||
|
||
; This test ensures that a library shader with a UAV does not get the module and | ||
; shader feature flag UAVsAtEveryStage when the DXIL validator version is >= 1.8 | ||
|
||
target triple = "dxil-pc-shadermodel6.5-library" | ||
|
||
; CHECK: Combined Shader Flags for Module | ||
; CHECK-NEXT: Shader Flags Value: 0x00000000 | ||
|
||
; CHECK-NOT: Note: shader requires additional functionality: | ||
; CHECK-NOT: UAVs at every shader stage | ||
|
||
; CHECK: Function test : 0x00000000 | ||
define void @test() "hlsl.export" { | ||
; RWBuffer<float> Buf : register(u0, space0) | ||
%buf0 = call target("dx.TypedBuffer", float, 1, 0, 1) | ||
@llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f32_1_0t( | ||
i32 0, i32 0, i32 1, i32 0, i1 false) | ||
ret void | ||
} | ||
|
||
!dx.valver = !{!1} | ||
!1 = !{i32 1, i32 8} | ||
|
||
!llvm.module.flags = !{!0} | ||
!0 = !{i32 1, !"dx.resmayalias", i32 1} |
36 changes: 36 additions & 0 deletions
36
llvm/test/CodeGen/DirectX/ShaderFlags/uavs-at-every-stage-vs.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,36 @@ | ||
; RUN: opt -S --passes="print-dx-shader-flags" 2>&1 %s | FileCheck %s | ||
; TODO: Remove this comment and add 'RUN' to the line below once vertex shaders are supported by llc | ||
; llc %s --filetype=obj -o - | obj2yaml | FileCheck %s --check-prefix=DXC | ||
|
||
; This test ensures that a Vertex shader with a UAV gets the module and | ||
; shader feature flag UAVsAtEveryStage | ||
|
||
target triple = "dxil-pc-shadermodel6.5-vertex" | ||
|
||
; CHECK: Combined Shader Flags for Module | ||
; CHECK-NEXT: Shader Flags Value: 0x00010000 | ||
|
||
; CHECK: Note: shader requires additional functionality: | ||
; CHECK: UAVs at every shader stage | ||
|
||
; CHECK: Function VSMain : 0x00000000 | ||
define void @VSMain() { | ||
; RWBuffer<float> Buf : register(u0, space0) | ||
%buf0 = call target("dx.TypedBuffer", float, 1, 0, 1) | ||
@llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f32_1_0t( | ||
i32 0, i32 0, i32 1, i32 0, i1 false) | ||
ret void | ||
} | ||
|
||
!dx.valver = !{!1} | ||
!1 = !{i32 1, i32 8} | ||
|
||
!llvm.module.flags = !{!0} | ||
!0 = !{i32 1, !"dx.resmayalias", i32 1} | ||
|
||
; DXC: - Name: SFI0 | ||
; DXC-NEXT: Size: 8 | ||
; DXC-NEXT: Flags: | ||
; DXC: UAVsAtEveryStage: true | ||
; DXC: NextUnusedBit: false | ||
; DXC: ... |
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.
Shouldnt this be only if the version is greater than 1.8, based on the issue description?
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.
If the version is less than 1.8, they would still get the UAVsAtEveryStage flag because they are not Compute or Pixel
Uh oh!
There was an error while loading. Please reload this page.
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 the real potential issue is if in the future we get new shader stage, the UAVsAtEveryStage flag will not get assigned to the new shader stage when the DXIL validator version is < 1.8
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.
@bogner Do you think the
hasUAVsAtEveryStage
function should be this instead?This is so that we don't have to edit the switch every time a new shader stage gets added.
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.
This is one of the places where the fact that we're using the triple environment rather than a dedicated enum is kind of annoying. If we had a dedicated enum then we would get a
-Wcovered-switch
warning if that came up and my answer to this question would unambiguously be no.However, I still don't think that this would be an improvement. Consider:
dxil-
triple at all), then calling this function doesn't make any sense. We could arguably make thedefault
casellvm_unreachable()
and just crash if that were to happen.