diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2020-04-07 13:56:01 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2020-04-07 13:56:01 -0700 |
| commit | ba232e44cbb016a4e7c111b42458394027369c5e (patch) | |
| tree | c7d56cfcc5e4c94d0d82358663d82014904ce854 | |
| parent | c5db04b8ba72d14d6bf2e20abf9c1b8d8466abc6 (diff) | |
Fix a bug around generic functions using DXR RayQuery (#1309)
The DXR `RayQuery` type is our first generic type defined in the stdlib that is marked as a target intrinsic but does *not* map to a custom `IRType` case. Instead, a reference to `RayQuery<T>` is encoded in the IR as an ordinary `specialize` instruction.
Unfortunately, this doesn't play nice with the current specialization logic, which considered a `specialize` instruction to not represent a "fully specialized" instruction, which then inhibits specialization of generics/functions that use such an instruction.
The fix here was to add more nuanced logic to the check for "fully specialized"-ness, so that it considers a `specialize` to already be fully specialized when the generic it applies to represents a target intrinsic.
I also added a note that the whole notion of "fully specialized"-ness that we use isn't really the right thing for the specialization pass, and how we really ought to use a notion of what is or is not a "value."
This change doesn't include a test because the only way to trigger the issue is using the DXR 1.1 `RayQuery` type, and that type is not supported in current release versions of DXC.
| -rw-r--r-- | source/slang/slang-ir-specialize.cpp | 58 |
1 files changed, 57 insertions, 1 deletions
diff --git a/source/slang/slang-ir-specialize.cpp b/source/slang/slang-ir-specialize.cpp index 6baf8fca5..d4f236138 100644 --- a/source/slang/slang-ir-specialize.cpp +++ b/source/slang/slang-ir-specialize.cpp @@ -366,6 +366,23 @@ struct SpecializationContext void maybeMarkAsFullySpecialized( IRInst* inst) { + // TODO: The logic here is completely bogus and + // we need to revisit the notion of fully-specialized-ness + // to only involve things that are semantically *values* + // rather than computations/expressions. + // + // The rules should be something like: + // + // * Literals are values + // * Composite type constructors where all the operands are value are values + // * References to nominal types are values + // * Built-in types where all the operands are values are values + // + // The system for defining value-ness probably needs + // to combine with the system for deduplicating instructions, + // since values are an important class of instruction we want + // to deduplicate. + switch(inst->op) { default: @@ -387,11 +404,50 @@ struct SpecializationContext // Certain instructions cannot ever be considered // fully specialized because they should never // be substituted into a generic as its arguments. - case kIROp_Specialize: case kIROp_lookup_interface_method: case kIROp_ExtractExistentialType: case kIROp_BindExistentialsType: break; + + case kIROp_Specialize: + // The `specialize` instruction is a bit sepcial, + // because it is possible to have a `specialize` + // of a built-in type so that it never gets + // substituted for another type. (e.g., the specific + // case where this code path first showed up + // as necessary was `RayQuery<>`) + // + { + auto specialize = cast<IRSpecialize>(inst); + auto base = specialize->getBase(); + if( auto generic = as<IRGeneric>(base) ) + { + // If the thing being specialized can be resolved, + // *and* it is a target intrinsic, ... + // + if( auto result = findGenericReturnVal(generic) ) + { + if( result->findDecoration<IRTargetIntrinsicDecoration>() ) + { + // ... then we should consider the instruction as + // "fully specialized" in the same cases as for + // any ordinary instruciton. + // + + if( areAllOperandsFullySpecialized(inst) ) + { + markInstAsFullySpecialized(inst); + } + return; + } + } + } + + // Otherwise, a `specialize` instruction falls into + // the case of instructions that should never be + // considered to be fully specialized. + } + break; } } |
