summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2021-01-29 13:17:51 -0800
committerGitHub <noreply@github.com>2021-01-29 13:17:51 -0800
commit0ab4d044896ad3d8c5c9cedbb7bd7ec94be81d69 (patch)
tree85a4e3995f99f0c7282bf12917f166eb3c4adab7
parentda6463af17798a0a72e2e0ecdc15e5b9b99e11f5 (diff)
Fix issue when passing ray query to a subroutine (#1680)
The problem would manifest for any code that declared a DXR 1.1 `RayQuery` value, but then only used it as one location in their code. The most common way for this to arise in user code was declaring a `RayQuery` and then handing it off to a helper/worker subroutine. RayQuery<0> myRayQuery; helperRoutine(myRayQuery, ...); The root cause was in the emit logic, where the initialization of `myRayQuery` above (a `defaultConstruct` operation in our IR) was getting folded into its (only) use site. This folding makes some sense, because the initialization of a ray query is not an operation with side effects, but doesn't work in practice because our way of handling default construction in HLSL output is by using a variable declaration. The simple fix here is to ensure that `defaultConstruct` instructions never get folded into use sites. If we decide to revisit the logic here, it might be possible to separate out the case where a `defaultConstruct` is being used as a stand-alone instruction, where we can emit it as: RayQuery<0> myRayQuery; versus cases where the `defaultConstruct` is being used as a sub-expression, such as: helperRoutine(RayQuery<0>(), ...); Whether or not we can emit the latter form (or if it would be equivalent) depends on details of how constructors like this are being implemented in dxc. For now it seems safest to emit things in a form that is obviously expected to work. Aside: Historically, the HLSL language has had no notion of "constructors" as being a thing. A variable that is declared but not initialized in HLSL has always been left uninitialized, since the first version of the language. The `RayQuery` type in DXR 1.1 is the first example of a type that appears to have a C++-style "default constructor," although HLSL as implemented by dxc still does not expose constructors as a user-visible or documented feature. (There is the small detail that the DXR 1.0 `HitGroup` type also relied on C++ constructor syntax, but I'm not aware of anybody using that feature right now, so it is mostly a curiosity.)
-rw-r--r--source/slang/slang-emit-c-like.cpp6
-rw-r--r--tests/pipeline/ray-tracing/ray-query-subroutine.slang40
-rw-r--r--tests/pipeline/ray-tracing/ray-query-subroutine.slang.hlsl26
3 files changed, 72 insertions, 0 deletions
diff --git a/source/slang/slang-emit-c-like.cpp b/source/slang/slang-emit-c-like.cpp
index 009ca17f9..d21e16186 100644
--- a/source/slang/slang-emit-c-like.cpp
+++ b/source/slang/slang-emit-c-like.cpp
@@ -894,6 +894,12 @@ bool CLikeSourceEmitter::shouldFoldInstIntoUseSites(IRInst* inst)
case kIROp_Alloca:
return false;
+ // Never fold these, because their result cannot be computed
+ // as a sub-expression (they must be emitted as a declaration
+ // or statement).
+ case kIROp_DefaultConstruct:
+ return false;
+
// Always fold these in, because they are trivial
//
case kIROp_IntLit:
diff --git a/tests/pipeline/ray-tracing/ray-query-subroutine.slang b/tests/pipeline/ray-tracing/ray-query-subroutine.slang
new file mode 100644
index 000000000..501071717
--- /dev/null
+++ b/tests/pipeline/ray-tracing/ray-query-subroutine.slang
@@ -0,0 +1,40 @@
+// ray-query-subroutine.slang
+
+// Regression test for passing a `RayQuery` value into a subroutine.
+
+//TEST:CROSS_COMPILE: -profile sm_6_5 -stage compute -entry computeMain -target dxil-assembly
+
+RWStructuredBuffer<int> gOutput;
+RaytracingAccelerationStructure gScene;
+
+int helper<let N : int>(RayQuery<N> q)
+{
+ RayDesc ray;
+ ray.Origin = 0;
+ ray.Direction = 0;
+ ray.TMin = 0;
+ ray.TMax = 1000.0;
+ q.TraceRayInline(
+ /* accellerationStructure: */ gScene,
+ /* rayFlags: */ N,
+ /* instanceInclusionmask: */ 0xFFFFFFFF,
+ /* ray: */ ray );
+
+ return 1;
+}
+
+[shader("compute")]
+void computeMain(uint tid : SV_DispatchThreadID)
+{
+ // Note: The original issue was due to "folding" of an instruction
+ // into use sites as part of emitting high-level-language code.
+ //
+ // In this case, the initial value of `rayQuery` has only a single
+ // use (in the call to `helper()`) and as a result was subject to
+ // "folding" during emit, because it had no side-effects.
+
+ RayQuery<0> rayQuery;
+ let result = helper(rayQuery);
+
+ gOutput[tid.x] = result;
+}
diff --git a/tests/pipeline/ray-tracing/ray-query-subroutine.slang.hlsl b/tests/pipeline/ray-tracing/ray-query-subroutine.slang.hlsl
new file mode 100644
index 000000000..5906823e5
--- /dev/null
+++ b/tests/pipeline/ray-tracing/ray-query-subroutine.slang.hlsl
@@ -0,0 +1,26 @@
+//TEST_IGNORE_FILE:
+
+RaytracingAccelerationStructure gScene_0 : register(t0);
+
+int helper_0(RayQuery<int(0) > q_0)
+{
+ RayDesc ray_0;
+ ray_0.Origin = (vector<float,3>) int(0);
+ ray_0.Direction = (vector<float,3>) int(0);
+ ray_0.TMin = (float) int(0);
+ ray_0.TMax = 1000.00000000000000000000;
+ q_0.TraceRayInline(gScene_0, (uint) int(0), (uint) int(-1), ray_0);
+ return int(1);
+}
+
+RWStructuredBuffer<int > gOutput_0 : register(u0);
+
+[shader("compute")][numthreads(1, 1, 1)]
+void computeMain(uint tid_0 : SV_DISPATCHTHREADID)
+{
+ RayQuery<int(0) > rayQuery_0;
+ int _S1 = helper_0(rayQuery_0);
+
+ gOutput_0[tid_0.x] = _S1;
+ return;
+}