Skip to content

[HLSL][DirectX] Avoid visited values when searching in hasNonUniformIndex #189498

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.

Sign up for GitHub

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
joaosaffran merged 13 commits into from
Apr 13, 2026
Merged

[HLSL][DirectX] Avoid visited values when searching in hasNonUniformIndex #189498

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
717dc6c
fix search to avoid alredy viset values
joaosaffran Mar 30, 2026
b97fd75
simplify test
joaosaffran Mar 30, 2026
6a382ea
rename test
joaosaffran Mar 30, 2026
494493b
improve comment wording
joaosaffran Mar 30, 2026
07fd932
add nuri version of the test
joaosaffran Mar 30, 2026
4b2d051
check immediate uses for nuri call
joaosaffran Mar 31, 2026
79d39c2
update logic so any path that pass through nuri leads to true
joaosaffran Apr 1, 2026
d50ab38
clean up
joaosaffran Apr 1, 2026
c644221
clean up
joaosaffran Apr 1, 2026
616c4e9
Merge branch 'main' of https://github.com/llvm/llvm-project into bugf…
joaosaffran Apr 8, 2026
223237d
Merge branch 'main' into bugfix/non_uniform_infinite_loop
joaosaffran Apr 9, 2026
07f9f79
remove recursion
joaosaffran Apr 9, 2026
a1be198
address commentes
joaosaffran-zz Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions llvm/lib/Target/DirectX/DXILOpLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,15 @@ class OpLowerer {
return false;

SmallVector<Value *> WorkList;
SmallPtrSet<Value *, 8> Visited;

WorkList.push_back(IndexOp);

while (!WorkList.empty()) {
Value *V = WorkList.pop_back_val();
if (!Visited.insert(V).second)
continue;

if (auto *CI = dyn_cast<CallInst>(V)) {
if (CI->getCalledFunction()->getIntrinsicID() ==
Intrinsic::dx_resource_nonuniformindex)
Expand Down
26 changes: 26 additions & 0 deletions llvm/test/CodeGen/DirectX/is_nonuniform_within_loop.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
; RUN: opt -S -dxil-intrinsic-expansion -dxil-op-lower -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s

; The goal of this test it to make sure compilation finishes successfully,
; in bad case it should timeout.

@bogner bogner Mar 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of just checking that we avoid a timeout, we should check that the NonUniformIndex argument to createHandle/createHandleFromBinding is set correctly to false. We should also have a version of the test that uses the index with @dx.resource.nonuniformindex that tests that we correctly set that to true even when we have cyclic users.

@hekota hekota Apr 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
; The goal of this test it to make sure compilation finishes successfully,
; in bad case it should timeout.
; Regression test for llvm/llvm-project#189438.
; The goal of this test it to make sure compilation finishes successfully.



@Out.str = private unnamed_addr constant [4 x i8] c"Out\00", align 1
; CHECK-LABEL: define void @main
; CHECK: call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 %phi, i1 false)

define void @main() local_unnamed_addr {
entry:
%cmp = icmp eq i32 4, 0
br i1 %cmp, label %_Z4mainj.exit, label %for.body.i

for.body.i: ; preds = %entry, %for.body.i
%phi = phi i32 [ %inc.loop, %for.body.i ], [ 0, %entry ]
%binding = tail call target("dx.RawBuffer", i32, 1, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 4, i32 %phi, ptr nonnull @Out.str)
%inc.counter = tail call noundef i32 @llvm.dx.resource.updatecounter(target("dx.RawBuffer", i32, 1, 0) %binding, i8 1)
%inc.loop = add nuw nsw i32 %phi, 1
%exitcond = icmp eq i32 %inc.loop, 4
br i1 %exitcond, label %_Z4mainj.exit, label %for.body.i

_Z4mainj.exit: ; preds = %for.body.i, %entry
ret void
}
27 changes: 27 additions & 0 deletions llvm/test/CodeGen/DirectX/is_nonuniform_within_loop_nuri.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
; RUN: opt -S -dxil-intrinsic-expansion -dxil-op-lower -mtriple=dxil-pc-shadermodel6.3-library %s | FileCheck %s

; The goal of this test it to make sure compilation finishes successfully,
; in bad case it should timeout.

@bogner bogner Mar 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test technically didn't hang, so this comment is a bit misleading. I also realize that it's a bit boring, in that we don't actually look at the phi node at all in this case. I wonder if a test based on something silly like this is a little bit more interesting, where the handlefrombinding still uses the value from the phi: https://hlsl.godbolt.org/z/TsaPzMhr5

Note that this never hung either, because when we see the function call we return early, but I think it's still a little bit interesting to have the case where we walk through the phi node at least once.



@Out.str = private unnamed_addr constant [4 x i8] c"Out\00", align 1
; CHECK-LABEL: define void @main
; CHECK: call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 %phi, i1 true)

define void @main() local_unnamed_addr {
entry:
%cmp = icmp eq i32 4, 0
br i1 %cmp, label %_Z4mainj.exit, label %for.body.i

for.body.i: ; preds = %entry, %for.body.i
%phi = phi i32 [ %inc.loop, %for.body.i ], [ 0, %entry ]
%nuri = tail call noundef i32 @llvm.dx.resource.nonuniformindex(i32 %phi)
%binding = tail call target("dx.RawBuffer", i32, 1, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 4, i32 %nuri, ptr nonnull @Out.str)
%inc.counter = tail call noundef i32 @llvm.dx.resource.updatecounter(target("dx.RawBuffer", i32, 1, 0) %binding, i8 1)
%inc.loop = add nuw nsw i32 %phi, 1
%exitcond = icmp eq i32 %inc.loop, 4
br i1 %exitcond, label %_Z4mainj.exit, label %for.body.i

_Z4mainj.exit: ; preds = %for.body.i, %entry
ret void
}
Loading