Unify handling of static and dynamic dispatch for interfaces (#1612)

Overview
========

Prior to this change, we had two different code generation strategies for interface/existential types in Slang, that didn't always play nicely together:

* The "legacy" static specialization approach could handle plugging in an arbitrary concrete type for an existential type parameter (including types with resources, etc.), but wouldn't work well with things like a `StructuredBuffer<>` of an interface type, and requires somewhat counter-intuitive layout rules to make work.

* The new dynamic dispatch approach produces simpler, more easily understood layouts by assuming that values of interface type can fit into a fixed number of bytes. The tradeoff there is that it cannot handle types that include resources (only POD types).

The goal of this change is to make it so that the two strategies can co-exist. In particular, in cases where a shader is amenable to both static specialization and dynamic dispatch, the type layouts should agree.

In order to make the type layouts agree, we:

* Declare that *all* values of existential type reserve storage according to the dynamic-dispatch rules (so 16 bytes for the RTTI and witness-table information, plus whatever bytes are needed to story "any value" of a conforming type).

* Then we modify the "legacy" layout rules so that if a value of concrete type can fit in the reserved "any value" space for a given interface, then it is laid out there exactly like the dynamic dispatch rules would do. Otherwise, we fall back to the previous legacy rules (since we don't need to agree with the dynamic-dispatch layout on types that can't be used with dynamic dispatch).

Details
=======

* Renamed `ExistentialBox` to `BoundInterfaceType` to better clarify how it relates to `BindExistentialsType`

* Unconditionally apply the `lowerGenerics` pass during emit, since it is now responsible for aspects of the lowering of existential types when specialization is used.

* Made IR type layout take the target into account, so that the layout of resource types can vary by target (e.g., being POD on some targets, and invalid on others)

* Cleaned up some issues around using global shader parameters as the "key" for their layout information in the global-scope layout (only comes up when there are global-scope `uniform` parameters)

* Made there be a default any-value size (16) instead of making it be an error to leave out. This was the simplest option; we could try to go back to having an error, but we'd need to only issue it if we are sure a type/interface is being used with dynamic dispatch, since static dispatch doesn't have to obey the restrictions.

* Changed lowering of existential types to tuples so that bound interfaces where the concrete type won't fit use a "pseudo-pointer" instead of an "any-value" to hold the payload

* Changed IR type legalization to handle the "pseudo-pointer" case and apply layout information from an interface type over to the payload part when static specialization was used.

* Changed some details of how witness tables were being lowered, so that we didn't have to create "proxy" witness tables for the constraints on associated types (just use the actual requirement entries we generate)

* Changed witness tables so that they know the subtype doing the conforming

* Added logic so that we don't generate pack/unpack logic and witness table wrapper functions for types that are incompatible with any-value/dynamic dispatch for a given interface.

* Changed the core AST-level type layout logic to use the dynamic-dispatch layout in case things fit, and the legacy static specialization case when things don't (while also reserving space for the dynamic-dispatch fields)

* Changed a bunch of test cases for static specialization to properly use the new layout (which introduces new buffers in some cases, and moves data around in others).

Future Work
===========

The experience of trying to reconcile our older way of handling interface-type specialization with our newer model (that supports dynamic dispatch) makes it clear that we really need to make similar changes to our handling of generic type parameters on entry points and at the global scope.

A future change should make it so that a global type parameter is lowered with a type layout similar to a value parameter of interface type, including the RTTI and witness-table pieces, and just leaving out the "any value" piece. A similar translation strategy should apply to entry-point generic parameters (mirroring how we lower generic functions for dynamic dispatch already), and value specialization parameters.

Co-authored-by: Yong He <yonghe@outlook.com>

1 files changed, 50 insertions, 48 deletions

diff --git a/source/slang/slang-ir-specialize.cpp b/source/slang/slang-ir-specialize.cpp
index efcdb4498..693494ac1 100644
--- a/source/slang/slang-ir-specialize.cpp
+++ b/source/slang/slang-ir-specialize.cpp
@@ -69,6 +69,16 @@ struct SpecializationContext
         //
         if(!inst) return true;
 
+        // An interface requirement entry should always be considered
+        // to be fully specialized, even if it hasn't been visited.
+        //
+        // Note: This logic is here to stop a circularity, where we
+        // can't mark an interface as used until its requirements are
+        // used, etc.
+        //
+        if(inst->op == kIROp_InterfaceRequirementEntry)
+            return true;
+
         return fullySpecializedInsts.Contains(inst);
     }
 
@@ -618,33 +628,6 @@ struct SpecializationContext
         return nullptr;
     }
 
-    void maybeInsertGetExistentialValue(IRInst* inst)
-    {
-        // If inst has `ExistentialBox` type, we need to make sure
-        // all uses are through `GetValueFromExistentialBox`.
-        if (auto existentialBoxType = as<IRExistentialBoxType>(inst->getDataType()))
-        {
-            ShortList<IRUse*> usesToReplace;
-            for (auto use = inst->firstUse; use; use = use->nextUse)
-            {
-                if (use->getUser()->op != kIROp_GetValueFromExistentialBox)
-                    usesToReplace.add(use);
-            }
-            for (auto use : usesToReplace)
-            {
-                auto user = use->getUser();
-                IRBuilder builderStorage;
-                auto builder = &builderStorage;
-                builder->sharedBuilder = &sharedBuilderStorage;
-                builder->setInsertBefore(user);
-                auto getValueInst = builder->emitGetValueFromExistentialBox(
-                    builder->getPtrType(existentialBoxType->getValueType()), inst);
-                use->set(getValueInst);
-                addToWorkList(getValueInst);
-            }
-        }
-    }
-
     // All of the machinery for generic specialization
     // has been defined above, so we will now walk
     // through the flow of the overall specialization pass.
@@ -712,10 +695,6 @@ struct SpecializationContext
             workListSet.Remove(inst);
             cleanInsts.Add(inst);
 
-            // If inst represents a value of ExistentialBox type, all its uses
-            // must be through a `GetValueFromExistentialBox` inst.
-            maybeInsertGetExistentialValue(inst);
-
             // For each instruction we process, we want to perform
             // a few steps.
             //
@@ -1131,8 +1110,33 @@ struct SpecializationContext
         // (which implicitly determines the concrete type), and
         // the witness table `w.
         //
-        if(as<IRMakeExistential>(inst))
+        if( auto makeExistential = as<IRMakeExistential>(inst) )
+        {
+            // We need to be careful about the type that we'd be specializing
+            // to, since it needs to be visible to both the caller and calee.
+            //
+            // In particular, if the type is the result of a function-local
+            // operation like `extractExistentialType`, then we can't possibly
+            // specialize the callee, since it wouldn't be able to access
+            // the same type (since the type is the result of an instruction in
+            // the body of the caller)
+            //
+            auto concreteVal = makeExistential->getWrappedValue();
+            auto concreteType = concreteVal->getDataType();
+
+            // TODO: We probably need/want a more robust test here.
+            // For now we are just listing the single opcode that is
+            // causing problems.
+            //
+            // TODO: eventually this check would become unnecessary because
+            // we can simply check if the `concreteType` is a compile-time
+            // constant value.
+            //
+            if(concreteType->op == kIROp_ExtractExistentialType)
+                return false;
+
             return true;
+        }
 
         // A `wrapExistential(v, T0,w0, T1, w1, ...)` instruction
         // is just a generalization of `makeExistential`, so it
@@ -1531,7 +1535,11 @@ struct SpecializationContext
     UInt calcExistentialBoxSlotCount(IRType* type)
     {
     top:
-        if( as<IRExistentialBoxType>(type) )
+        if( as<IRBoundInterfaceType>(type) )
+        {
+            return 2;
+        }
+        else if( as<IRInterfaceType>(type) )
         {
             return 2;
         }
@@ -1793,14 +1801,16 @@ struct SpecializationContext
             auto index = inst->getIndex();
 
             auto val = wrapInst->getWrappedValue();
+            auto ptrType = cast<IRPtrTypeBase>(val->getDataType());
+            auto arrayType = cast<IRArrayTypeBase>(ptrType->getValueType());
+            auto elementType = arrayType->getElementType();
+
             auto resultType = inst->getFullType();
 
             IRBuilder builder;
             builder.sharedBuilder = &sharedBuilderStorage;
             builder.setInsertBefore(inst);
 
-            auto elementType = cast<IRArrayTypeBase>(val->getDataType())->getElementType();
-
             List<IRInst*> slotOperands;
             UInt slotOperandCount = wrapInst->getSlotOperandCount();
 
@@ -1809,7 +1819,8 @@ struct SpecializationContext
                 slotOperands.add(wrapInst->getSlotOperand(ii));
             }
 
-            auto newElementAddr = builder.emitElementAddress(elementType, val, index);
+            auto elementPtrType = builder.getPtrType(ptrType->op, elementType);
+            auto newElementAddr = builder.emitElementAddress(elementPtrType, val, index);
 
             auto newWrapExistentialInst = builder.emitWrapExistential(
                 resultType, newElementAddr, slotOperandCount, slotOperands.getBuffer());
@@ -1870,24 +1881,15 @@ struct SpecializationContext
 
         if( auto baseInterfaceType = as<IRInterfaceType>(baseType) )
         {
-            // A `BindExistentials<ISomeInterface, ConcreteType, ...>` can
-            // just be simplified to `ExistentialBox<ConcreteType>`.
-            //
-            // Note: We do *not* simplify straight to `ConcreteType`, because
-            // that would mess up the layout for aggregate types that
-            // contain interfaces. The logical indirection introduced
-            // by `ExistentialBox<...>` will be handled by a later type
-            // legalization pass that moved the type "pointed to" by
-            // the box out of line from other fields.
-
             // We always expect two slot operands, one for the concrete type
             // and one for the witness table.
             //
             SLANG_ASSERT(slotOperandCount == 2);
-            if(slotOperandCount <= 1) return;
+            if(slotOperandCount < 2) return;
 
             auto concreteType = (IRType*) type->getExistentialArg(0);
-            auto newVal = builder.getExistentialBoxType(concreteType, baseInterfaceType);
+            auto witnessTable = type->getExistentialArg(1);
+            auto newVal = builder.getBoundInterfaceType(baseInterfaceType, concreteType, witnessTable);
 
             addUsersToWorkList(type);
             type->replaceUsesWith(newVal);