-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[HLSL] Apply NoRecurse attrib to all HLSL functions #105907
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
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
norecurse
attribute apply to direct recursion only or indirectly as well (e.g.A->B->A
). We do have bounded recursion in raytracing in thatTraceRay
can invoke aClosetHit
shader 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.