From ba232e44cbb016a4e7c111b42458394027369c5e Mon Sep 17 00:00:00 2001
From: Tim Foley <tfoleyNV@users.noreply.github.com>
Date: Tue, 7 Apr 2020 13:56:01 -0700
Subject: 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.
---
 source/slang/slang-ir-specialize.cpp | 58 +++++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

(limited to 'source')

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;
         }
     }
 
-- 
cgit v1.2.3