[HLSL] Apply NoRecurse attrib to all HLSL functions #105907
There are no files selected for viewing
This file contains 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 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: %clang_cc1 -x hlsl -triple dxil-pc-shadermodel6.3-library -finclude-default-header %s -emit-llvm -disable-llvm-passes -o - | FileCheck %s | ||
| // RUN: %clang_cc1 -x hlsl -triple dxil-pc-shadermodel6.0-compute -finclude-default-header %s -emit-llvm -disable-llvm-passes -o - | FileCheck %s | ||
|
|
||
| // Verify that a few different function types all get the NoRecurse attribute | ||
|
|
||
| #define MAX 100 | ||
|
|
||
| struct Node { | ||
| uint value; | ||
| uint key; | ||
| uint left, right; | ||
| }; | ||
|
|
||
| // CHECK: Function Attrs:{{.*}}norecurse | ||
| // CHECK: define noundef i32 @"?Find@@YAIY0GE@UNode@@I@Z"(ptr noundef byval([100 x %struct.Node]) align 4 %SortedTree, i32 noundef %key) [[IntAttr:\#[0-9]+]] | ||
| // CHECK: ret i32 | ||
| // Find and return value corresponding to key in the SortedTree | ||
| uint Find(Node SortedTree[MAX], uint key) { | ||
|
There was a problem hiding this comment. Do we need such complicated functions for this test? Does the body matter here or is it really just the internal/external properties that matter. In my opinion, the function body is distracting from the essence of the test here. There was a problem hiding this comment. Probably not. I admit I got carried away, but in the process I discovered the constraints of the current implementation and potentially introduced some incidentals that might catch future issues by creating a more representative shader. I realize that philosophies might differ here, but I'm reluctant to change it. |
||
| uint nix = 0; // head | ||
| while(true) { | ||
| if (nix < 0) | ||
| return 0.0; // Not found | ||
| Node n = SortedTree[nix]; | ||
| if (n.key == key) | ||
| return n.value; | ||
| if (key < n.key) | ||
| nix = n.left; | ||
| else | ||
| nix = n.right; | ||
| } | ||
| } | ||
|
|
||
| // CHECK: Function Attrs:{{.*}}norecurse | ||
| // CHECK: define noundef i1 @"?InitTree@@YA_NY0GE@UNode@@V?$RWBuffer@T?$__vector@I$03@__clang@@@hlsl@@I@Z"(ptr noundef byval([100 x %struct.Node]) align 4 %tree, ptr noundef byval(%"class.hlsl::RWBuffer") align 4 %encodedTree, i32 noundef %maxDepth) [[ExtAttr:\#[0-9]+]] | ||
| // CHECK: ret i1 | ||
| // Initialize tree with given buffer | ||
| // Imagine the inout works | ||
| export | ||
| bool InitTree(/*inout*/ Node tree[MAX], RWBuffer<uint4> encodedTree, uint maxDepth) { | ||
| uint size = pow(2.f, maxDepth) - 1; | ||
| if (size > MAX) return false; | ||
| for (uint i = 1; i < size; i++) { | ||
| tree[i].value = encodedTree[i].x; | ||
| tree[i].key = encodedTree[i].y; | ||
| tree[i].left = encodedTree[i].z; | ||
| tree[i].right = encodedTree[i].w; | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| RWBuffer<uint4> gTree; | ||
|
|
||
| // Mangled entry points are internal | ||
| // CHECK: Function Attrs:{{.*}}norecurse | ||
| // CHECK: define internal void @"?main@@YAXI@Z"(i32 noundef %GI) [[IntAttr]] | ||
| // CHECK: ret void | ||
|
|
||
| // Canonical entry points are external and shader attributed | ||
| // CHECK: Function Attrs:{{.*}}norecurse | ||
| // CHECK: define void @main() [[EntryAttr:\#[0-9]+]] | ||
| // CHECK: ret void | ||
|
|
||
| [numthreads(1,1,1)] | ||
| [shader("compute")] | ||
| void main(uint GI : SV_GroupIndex) { | ||
| Node haystack[MAX]; | ||
| uint needle = 0; | ||
| if (InitTree(haystack, gTree, GI)) | ||
| needle = Find(haystack, needle); | ||
| } | ||
|
|
||
| // Mangled entry points are internal | ||
| // CHECK: Function Attrs:{{.*}}norecurse | ||
| // CHECK: define internal void @"?defaultMain@@YAXXZ"() [[IntAttr]] | ||
| // CHECK: ret void | ||
|
|
||
| // Canonical entry points are external and shader attributed | ||
| // CHECK: Function Attrs:{{.*}}norecurse | ||
| // CHECK: define void @defaultMain() [[EntryAttr]] | ||
| // CHECK: ret void | ||
|
|
||
| [numthreads(1,1,1)] | ||
| [shader("compute")] | ||
| void defaultMain() { | ||
| Node haystack[MAX]; | ||
| uint needle = 0; | ||
| if (InitTree(haystack, gTree, 4)) | ||
| needle = Find(haystack, needle); | ||
| } | ||
|
|
||
| // CHECK: attributes [[IntAttr]] = {{.*}} norecurse | ||
| // CHECK: attributes [[ExtAttr]] = {{.*}} norecurse | ||
| // CHECK: attributes [[EntryAttr]] = {{.*}} norecurse | ||
This file contains 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 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
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.
Does the
norecurseattribute apply to direct recursion only or indirectly as well (e.g.A->B->A). We do have bounded recursion in raytracing in thatTraceRaycan invoke aClosetHitshader which can then again callTraceRay.I don't think that changes anything about this PR, but just throwing it out there in case it matters.
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 attribute is descriptive and used in a few places to determine if recursion is expected or not. There is not actually any place that checks for recursion and produces an error for HLSL nor for OpenCL. There is a check for it when inlining takes place, but that's too late for diagnostics. #105244 is meant to address this problem.