From f161686e8e260a4b0e6e0a773cf1cf16069f41bf Mon Sep 17 00:00:00 2001 From: Theresa Foley <10618364+tangent-vector@users.noreply.github.com> Date: Tue, 13 Jun 2023 14:40:02 -0700 Subject: Fixes for Shader Execution Reordering on VK (#2929) * Fixes for Shader Execution Reordering on VK There are some mismatches between the way that hit objects are handled between the current NVAPI/HLSL and proposed GLSL extensions for shader execution reordering. These mismatches create complications for generating valid GLSL/SPIR-V code from input Slang. Many of the problems that apply to `HitObject` also apply to the existing `RayQuery<>` type used for "inline" ray tracing. In the case of `RayQuery<>` we have that for *both* HLSL and GLSL/SPIR-V: * A `RayQuery` (or `rayQueryEXT`) is an opaque handle to underlying mutable storage * The storage that backs a `RayQuery` is allocated as part of the "defualt constructor" for a local variable declared with type `RayQuery`. * The `RayQuery` API provides numerous operations that mutate the storage referred to by the opaque handle. The key difference between HLSL and GLSL/SPIR-V for the case of a `RayQuery` amounts to: * In HLSL, local variables of type `RayQuery` can be assigned to, and assignment has by-reference semantics. It is possible to create multiple aliased handles to the same underlying storage. * In GLSL/SPIR-V, local variables of type `rayQueryEXT` cannot be assigned to, returned from functions, etc. It is impossible to create multiple aliased handles to the same underlying storage. The case for `HitObject`s is signicantly *more* messy, because: * In NVAPI/HLSL a `HitObject` is effectively a "value type" in that it only exposes constructors, and there is no way to mutate the state of a `HitObject` other than by assignment to a variable of that type. It makes no semantic difference whether a `HitObject` directly stores the value(s), or if it is a handle, since there is no way to introduce aliasing of mutable state. Assignment of `HitObject`s semantically creates a copy. * In GLSL/SPIR-V, a `hitObjectNV` is, like a `rayQueryEXT`, a handle to underlying mutable state. These handles cannot be assigned, returned from functions, etc. There is no way to make a copy of a hit object. This change includes several changes to how *both* `RayQuery<>` and `HitObject` are implemented, with the intention of getting more cases to work correctly when compiling for GLSL/SPIR-V, and to set up a more clear mental model for the semantics we want to give to these types in Slang, and how those semantics can/should map to our targets. An overview of important changes: * Marked a few operations on `RayQuery` as `[mutating]` that realistically should have already been that way. * Marked the `HitObject` type as being non-copyable (an attribute we do not currently enforce), and marked the various GLSL operations that construct a hit object as having an `out` parameter of the `HitObject` type (even if they are nominally specified in GLSL as not writing to the correspondign parameter). * Added a distinct IR opcode (`allocateOpaqueHandle`) to represent the implicit allocation that happens when declaring a variable of type `HitObject` or `RayQuery`, and made the "implicit constructor" for those types map to the new op. This operation took a lot of tweaking to get emitting in a reasonable way, and I'm still not 100% sure that all of the emission-related logic for it is strictly required (or correct). * Added new IR instructions for `HitObject` and `RayQuery` types, and made the stdlib types map to those IR instructions. * Treat `HitObject` and `RayQuery` as resource types for the purpose of our existing pass that specializes calls to functions that have outputs of resource type * Added a new test case that includes a function that returns a `HitObject` as its result. * Many test cases saw slight changes in their output (especially around the relative ordering of declarations of `HitObject`s and `RayQuery`s with other instructions) * Remove debugging logic --- source/slang/slang-emit-glsl.cpp | 65 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) (limited to 'source/slang/slang-emit-glsl.cpp') diff --git a/source/slang/slang-emit-glsl.cpp b/source/slang/slang-emit-glsl.cpp index bc170dadc..a86971bf5 100644 --- a/source/slang/slang-emit-glsl.cpp +++ b/source/slang/slang-emit-glsl.cpp @@ -869,6 +869,61 @@ void GLSLSourceEmitter::emitLoopControlDecorationImpl(IRLoopControlDecoration* d } } +void GLSLSourceEmitter::_emitInstAsVarInitializerImpl(IRInst* inst) +{ + // Some opcodes can be folded into a variable initialization + // by allowing the variable to be "default-constructed." + // + switch (inst->getOp()) + { + case kIROp_AllocateOpaqueHandle: + // + // Note: semantically, we should only elide the initializer + // if `inst` is able to be folded here, since otherwise + // it could be a single allocation that is used to initialize + // multiple local variables (which should then alias the + // same location). + // + // However, since GlSL doesn't support assignment of opaque + // handle types, code will fail to compile downstream in + // the case where the initializer *doesn't* fold. + // + // The decision being made here should help ensure that we + // don't emit code that silently has different semantics + // than the input. + // + if (shouldFoldInstIntoUseSites(inst)) + { + return; + } + break; + + default: + break; + } + + // We fall back to the default behavior for all targets, + // which is to emit `inst` as an initial-value expression + // after an `=`. + // + Super::_emitInstAsVarInitializerImpl(inst); +} + +void GLSLSourceEmitter::_emitStoreImpl(IRStore* store) +{ + auto srcVal = store->getVal(); + switch (srcVal->getOp()) + { + default: + Super::_emitStoreImpl(store); + break; + + case kIROp_AllocateOpaqueHandle: + break; + } + +} + void GLSLSourceEmitter::_emitSpecialFloatImpl(IRType* type, const char* valueExpr) { if( type->getOp() != kIROp_FloatType ) @@ -2153,6 +2208,16 @@ void GLSLSourceEmitter::emitSimpleTypeImpl(IRType* type) m_writer->emit("int"); return; } + case kIROp_RayQueryType: + { + m_writer->emit("rayQueryEXT"); + return; + } + case kIROp_HitObjectType: + { + m_writer->emit("hitObjectNV"); + return; + } default: break; } -- cgit v1.2.3