Make AST and IR share type legalization code (#303)

* Make AST and IR share type legalization code

A previous change already made it so that the AST-to-AST lowering/legalization pass could work together with IR-based lowering of `import`ed code, but that change didn't take into account the case where a function written in the AST needed to call an IR function and pass in a type that required legalization.

Both the IR-based and AST-based passes had their own approaches to type legalization, that mostly agreed on the desired output, but they ended up creating their own representations for legalized types which would mean that for a function call the caller and callee might end up legalizing the parameter list to use different types.

This change tries to fix this issue (and adds a new test case that relies on the fix) by massively overhauling the AST-based legalization pass so that it uses the same type legalization code as the IR. The shared code has been moved out into `legalize-types.{h,cpp}`.

Notes:

- I eliminated the `FilteredTupleType` type, since it was starting to cause code duplication in a lot of places. Instead, type legalization just creates new `struct` types to represent the result of filtering.

  - One big consequence of this is that the `LegalType::pair` case needs to remember for each field in the original type which field (if any) in the new `struct` type it maps to

  - A big source of complexity (and probably bugs) in this code is trying to figure out how to parent these new `struct` definitions effectively. A good follow-on change would be something that outputs declarations on-demand during the AST emit logic (as we do for the IR), just to avoid some of this song and dance.

- The old AST type legalization had a notion of both a "tuple" type and a "varying tuple" type. The "tuple" case was quite complex, and combined behavior currently handled by `LegalType::pair` (for splitting into ordinary and special sides) and `LegalType::tuple` (for holding multiple distinct elements to represent the fields of an aggregate). The "varying tuple" case was closer to `LegalType::tuple`, so I tried to just re-use the existing logic for that too. The one place this potentially gets messy is in `reifyTuple()`.

  - The messiest bit of handling the "varying tuple" concept (which is used for GLSL shader inputs/outputs since they have to be scalarized) is that when passing them as function arguments we need to reify the tuple back into a structured value. Because the `LegalExpr` hierarchy doesn't have type information, but constructing a value of the "original" type requires such information, things get a little messy.

- I did *not* try to deal with any of the logic related to handling system inputs/outputs for cross-compilation purposes. Of course, the long-term goal is that any actual cross-compilation is handled via the IR, but this change can't afford to break the AST-based path just yet. As a result, there is still quite a bit of complexity in the handling of assignment, to deal with cases where "fixups" are required.

* fixup: bad code in macro, not caught by Visual Studio compiler

* fixup: more stuff missed by VS compiler

* fixup: VS continutes to miss stuff in UNREACHABLE_RETURN

1 files changed, 133 insertions, 123 deletions

diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp
index 4a084c714..b9aa3c027 100644
--- a/source/slang/emit.cpp
+++ b/source/slang/emit.cpp
@@ -3,6 +3,7 @@
 
 #include "ast-legalize.h"
 #include "ir-insts.h"
+#include "legalize-types.h"
 #include "lower-to-ir.h"
 #include "mangle.h"
 #include "name.h"
@@ -1236,13 +1237,6 @@ struct EmitVisitor
         emitTypeImpl(type->valueType, arg.declarator);
     }
 
-    void visitFilteredTupleType(FilteredTupleType* type, TypeEmitArg const& arg)
-    {
-        auto declarator = arg.declarator;
-        emit(getMangledTypeName(type));
-        EmitDeclarator(declarator);
-    }
-
     void EmitType(
         RefPtr<Type>        type,
         SourceLoc const&    typeLoc,
@@ -2952,7 +2946,7 @@ struct EmitVisitor
             Decl* decl = declRef.getDecl();
             if(irDeclSet->Contains(decl))
             {
-                emit(getMangledName(declRef));
+                emit(getIRName(declRef));
                 return;
             }
         }
@@ -3605,10 +3599,6 @@ struct EmitVisitor
         // The data type that describes where stuff in the constant buffer should go
         RefPtr<Type> dataType = parameterGroupType->elementType;
 
-        // We expect/require the data type to be a user-defined `struct` type
-        auto declRefType = dataType->As<DeclRefType>();
-        SLANG_RELEASE_ASSERT(declRefType);
-
         // We expect to always have layout information
         layout = maybeFetchLayout(varDecl, layout);
         SLANG_RELEASE_ASSERT(layout);
@@ -3642,27 +3632,48 @@ struct EmitVisitor
         emitHLSLRegisterSemantic(*info);
 
         Emit("\n{\n");
-        if (auto structRef = declRefType->declRef.As<StructDecl>())
-        {
-            int fieldCounter = 0;
 
-            for (auto field : getMembersOfType<StructField>(structRef))
+        // We expect the data type to be a user-defined `struct` type,
+        // but it might also be a "filtered" type that represents the
+        // case where only some fields of the original type are valid
+        // to appear inside of a `struct`.
+        if (auto declRefType = dataType->As<DeclRefType>())
+        {
+            if (auto structRef = declRefType->declRef.As<StructDecl>())
             {
-                int fieldIndex = fieldCounter++;
+                int fieldCounter = 0;
 
-                emitVarDeclHead(field);
+                for (auto field : getMembersOfType<StructField>(structRef))
+                {
+                    int fieldIndex = fieldCounter++;
+
+                    // Skip fields that have `void` type, since these represent
+                    // declarations that got legalized out of existence.
+                    if(GetType(field)->Equals(getSession()->getVoidType()))
+                        continue;
 
-                RefPtr<VarLayout> fieldLayout = structTypeLayout->fields[fieldIndex];
-                SLANG_RELEASE_ASSERT(fieldLayout->varDecl.GetName() == field.GetName());
+                    emitVarDeclHead(field);
 
-                // Emit explicit layout annotations for every field
-                emitHLSLParameterGroupFieldLayoutSemantics(layout, fieldLayout);
+                    RefPtr<VarLayout> fieldLayout = structTypeLayout->fields[fieldIndex];
+                    SLANG_RELEASE_ASSERT(fieldLayout->varDecl.GetName() == field.GetName());
 
-                emitVarDeclInit(field);
+                    // Emit explicit layout annotations for every field
+                    emitHLSLParameterGroupFieldLayoutSemantics(layout, fieldLayout);
 
-                Emit(";\n");
+                    emitVarDeclInit(field);
+
+                    Emit(";\n");
+                }
+            }
+            else
+            {
+                SLANG_UNEXPECTED("unexpected element type for parameter group");
             }
         }
+        else
+        {
+            SLANG_UNEXPECTED("unexpected element type for parameter group");
+        }
         Emit("}\n");
     }
 
@@ -3773,10 +3784,6 @@ struct EmitVisitor
         // The data type that describes where stuff in the constant buffer should go
         RefPtr<Type> dataType = parameterGroupType->elementType;
 
-        // We expect/require the data type to be a user-defined `struct` type
-        auto declRefType = dataType->As<DeclRefType>();
-        SLANG_RELEASE_ASSERT(declRefType);
-
         // We expect the layout, if present, to be for a structured type...
         RefPtr<StructTypeLayout> structTypeLayout;
         if (layout)
@@ -3827,26 +3834,54 @@ struct EmitVisitor
         }
 
         Emit("\n{\n");
-        if (auto structRef = declRefType->declRef.As<StructDecl>())
+
+        // We expect the data type to be a user-defined `struct` type,
+        // but it might also be a "filtered" type that represents the
+        // case where only some fields of the original type are valid
+        // to appear inside of a `struct`.
+        if (auto declRefType = dataType->As<DeclRefType>())
         {
-            for (auto field : getMembersOfType<StructField>(structRef))
+
+            if (auto structRef = declRefType->declRef.As<StructDecl>())
             {
-                if (structTypeLayout)
+                int fieldCounter = 0;
+                for (auto field : getMembersOfType<StructField>(structRef))
                 {
-                    RefPtr<VarLayout> fieldLayout;
-                    structTypeLayout->mapVarToLayout.TryGetValue(field.getDecl(), fieldLayout);
-                    //            assert(fieldLayout);
+                    int fieldIndex = fieldCounter++;
+
+                    // Skip fields that have `void` type, since these represent
+                    // declarations that got legalized out of existence.
+                    if(GetType(field)->Equals(getSession()->getVoidType()))
+                        continue;
+
+                    if (structTypeLayout)
+                    {
+                        RefPtr<VarLayout> fieldLayout = structTypeLayout->fields[fieldIndex];
+                        //            assert(fieldLayout);
+
+                        // TODO(tfoley): We may want to emit *some* of these,
+                        // some of the time...
+            //            emitGLSLLayoutQualifiers(fieldLayout);
+                    }
 
-                    // TODO(tfoley): We may want to emit *some* of these,
-                    // some of the time...
-        //            emitGLSLLayoutQualifiers(fieldLayout);
-                }
 
-                EmitVarDeclCommon(field);
+                    EmitVarDeclCommon(field);
 
-                Emit(";\n");
+                    Emit(";\n");
+                }
+            }
+            else
+            {
+                SLANG_UNEXPECTED("unexpected element type for parameter group");
             }
         }
+        else
+        {
+            SLANG_UNEXPECTED("unexpected element type for parameter group");
+        }
+
+
+
         Emit("}");
 
         if( varDecl->getNameLoc().isValid() )
@@ -3890,13 +3925,10 @@ struct EmitVisitor
             }
         }
 
-        // Skip fields that have been tuple-ified and don't contribute
-        // any fields of "ordinary" type.
-        if (auto tupleFieldMod = decl->FindModifier<TupleFieldModifier>())
-        {
-            if (!tupleFieldMod->hasAnyNonTupleFields)
-                return;
-        }
+        // Skip fields that have `void` type, since these may be introduced
+        // as part of type leglaization.
+        if(decl->getType()->Equals(getSession()->getVoidType()))
+            return;
 
         RefPtr<VarLayout> layout = arg.layout;
         layout = maybeFetchLayout(decl, layout);
@@ -4129,16 +4161,6 @@ emitDeclImpl(decl, nullptr);
 
     String getIRName(Decl* decl)
     {
-        // It is a bit ugly, but we need a deterministic way
-        // to get a name for things when emitting from the IR
-        // that won't conflict with any keywords, builtins, etc.
-        // in the target language.
-        //
-        // Eventually we should accomplish this by using
-        // mangled names everywhere, but that complicates things
-        // when we are also using direct comparison to fxc/glslang
-        // output for some of our tests.
-        //
         // TODO: need a flag to get rid of the step that adds
         // a prefix here, so that we can get "clean" output
         // when needed.
@@ -4155,7 +4177,27 @@ emitDeclImpl(decl, nullptr);
 
     String getIRName(DeclRefBase const& declRef)
     {
-        return getIRName(declRef.decl);
+        // It is a bit ugly, but we need a deterministic way
+        // to get a name for things when emitting from the IR
+        // that won't conflict with any keywords, builtins, etc.
+        // in the target language.
+        //
+        // Eventually we should accomplish this by using
+        // mangled names everywhere, but that complicates things
+        // when we are also using direct comparison to fxc/glslang
+        // output for some of our tests.
+        //
+
+        String name;
+        if (context->shared->entryPoint->compileRequest->compileFlags & SLANG_COMPILE_FLAG_NO_MANGLING)
+        {
+            name.append(getText(declRef.GetName()));
+        }
+        else
+        {
+            name.append(getMangledName(declRef));
+        }
+        return name;
     }
 
     String getGLSLSystemValueName(
@@ -6279,9 +6321,12 @@ emitDeclImpl(decl, nullptr);
 
                     auto fieldLayout = structTypeLayout->fields[fieldIndex++];
 
+                    auto fieldType = GetType(ff);
+                    if(fieldType->Equals(getSession()->getVoidType()))
+                        continue;
+
                     emitIRVarModifiers(ctx, fieldLayout);
 
-                    auto fieldType = GetType(ff);
                     emitIRType(ctx, fieldType, getIRName(ff));
 
                     emitHLSLParameterGroupFieldLayoutSemantics(layout, fieldLayout);
@@ -6290,26 +6335,6 @@ emitDeclImpl(decl, nullptr);
                 }
             }
         }
-        else if (auto filteredTupleType = elementType->As<FilteredTupleType>())
-        {
-            auto structTypeLayout = typeLayout.As<StructTypeLayout>();
-            assert(structTypeLayout);
-
-            for (auto ee : filteredTupleType->elements)
-            {
-                RefPtr<VarLayout> fieldLayout;
-                structTypeLayout->mapVarToLayout.TryGetValue(ee.fieldDeclRef, fieldLayout);
-
-                emitIRVarModifiers(ctx, fieldLayout);
-
-                auto fieldType = ee.type;
-                emitIRType(ctx, fieldType, getIRName(ee.fieldDeclRef));
-
-                emitHLSLParameterGroupFieldLayoutSemantics(layout, fieldLayout);
-
-                emit(";\n");
-            }
-        }
         else
         {
             emit("/* unexpected */");
@@ -6376,9 +6401,12 @@ emitDeclImpl(decl, nullptr);
 
                     auto fieldLayout = structTypeLayout->fields[fieldIndex++];
 
+                    auto fieldType = GetType(ff);
+                    if(fieldType->Equals(getSession()->getVoidType()))
+                        continue;
+
                     emitIRVarModifiers(ctx, fieldLayout);
 
-                    auto fieldType = GetType(ff);
                     emitIRType(ctx, fieldType, getIRName(ff));
 
 //                    emitHLSLParameterGroupFieldLayoutSemantics(layout, fieldLayout);
@@ -6589,7 +6617,7 @@ emitDeclImpl(decl, nullptr);
         }
 
         Emit("struct ");
-        emit(declRef.GetName());
+        EmitDeclRef(declRef);
         Emit("\n{\n");
         for( auto ff : GetFields(declRef) )
         {
@@ -6597,6 +6625,11 @@ emitDeclImpl(decl, nullptr);
                 continue;
 
             auto fieldType = GetType(ff);
+
+            // Skip `void` fields that might have been created by legalization.
+            if(fieldType->Equals(getSession()->getVoidType()))
+                continue;
+
             emitIRType(ctx, fieldType, getIRName(ff));
 
             EmitSemantics(ff.getDecl());
@@ -6655,43 +6688,6 @@ emitDeclImpl(decl, nullptr);
                 ensureStructDecl(ctx, structDeclRef);
             }
         }
-        else if (auto filteredTupleType = type->As<FilteredTupleType>())
-        {
-            // First, ensure that the element types are ready:
-            for (auto ee : filteredTupleType->elements)
-            {
-                if (ee.type)
-                {
-                    emitIRUsedType(ctx, ee.type);
-                }
-            }
-
-            // Now, we want to ensure we've emitted a
-            // matching `struct` type declaration.
-
-            String mangledName = getMangledTypeName(filteredTupleType);
-            if (!ctx->shared->irTupleTypes.Contains(mangledName))
-            {
-                ctx->shared->irTupleTypes.Add(mangledName);
-
-                // Emit the damn `struct` decl...
-
-                Emit("struct ");
-                emit(mangledName);
-                Emit("\n{\n");
-                for( auto ee : filteredTupleType->elements )
-                {
-                    if (!ee.type)
-                        continue;
-
-                    emitIRType(ctx, ee.type, getIRName(ee.fieldDeclRef));
-
-                    emit(";\n");
-                }
-                Emit("};\n");
-
-            }
-        }
         else
         {}
     }
@@ -6846,7 +6842,8 @@ StructTypeLayout* getGlobalStructLayout(
 }
 
 void legalizeTypes(
-    IRModule*   module);
+    TypeLegalizationContext*    context,
+    IRModule*                   module);
 
 String emitEntryPoint(
     EntryPointRequest*  entryPoint,
@@ -6937,6 +6934,9 @@ String emitEntryPoint(
     // Next we will check for case (2a):
     else if (!(translationUnit->compileRequest->compileFlags & SLANG_COMPILE_FLAG_USE_IR))
     {
+        TypeLegalizationContext typeLegalizationContext;
+        typeLegalizationContext.session = entryPoint->compileRequest->mSession;
+
         // This case means the user has opted out of using the IR (so we can't use the
         // cases below), but they either turned on semantic checking *or* imported some
         // Slang code, so they can't use the case above.
@@ -6958,7 +6958,8 @@ String emitEntryPoint(
             programLayout,
             target,
             &sharedContext.extensionUsageTracker,
-            nullptr);
+            nullptr,
+            &typeLegalizationContext);
         sharedContext.program = lowered.program;
 
         // Note that we emit the main body code of the program *before*
@@ -6976,6 +6977,9 @@ String emitEntryPoint(
     // are certain steps that need to be shared.
     else
     {
+        TypeLegalizationContext typeLegalizationContext;
+        typeLegalizationContext.session = entryPoint->compileRequest->mSession;
+
         // We are going to create a fresh IR module that we will use to
         // clone any code needed by the user's entry point.
         IRSpecializationState* irSpecializationState = createIRSpecializationState(
@@ -6985,6 +6989,8 @@ String emitEntryPoint(
             targetRequest);
         IRModule* irModule = getIRModule(irSpecializationState);
 
+        typeLegalizationContext.irModule = irModule;
+
         LoweredEntryPoint lowered;
         if(translationUnit->compileFlags & SLANG_COMPILE_FLAG_NO_CHECKING)
         {
@@ -7002,7 +7008,8 @@ String emitEntryPoint(
                 programLayout,
                 target,
                 &sharedContext.extensionUsageTracker,
-                irSpecializationState);
+                irSpecializationState,
+                &typeLegalizationContext);
         }
         else
         {
@@ -7044,7 +7051,9 @@ String emitEntryPoint(
         // we need to ensure that the code only uses types
         // that are legal on the chosen target.
         // 
-        legalizeTypes(irModule);
+        legalizeTypes(
+            &typeLegalizationContext,
+            irModule);
 
         //  Debugging output of legalization
 #if 0
@@ -7053,6 +7062,8 @@ String emitEntryPoint(
         fprintf(stderr, "###\n");
 #endif
 
+        sharedContext.irDeclSetForAST = &lowered.irDecls;
+
         // After all of the required optimization and legalization
         // passes have been performed, we can emit target code from
         // the IR module.
@@ -7065,7 +7076,6 @@ String emitEntryPoint(
         // that we need to output, we'll do it now.
         if (translationUnit->compileFlags & SLANG_COMPILE_FLAG_NO_CHECKING)
         {
-            sharedContext.irDeclSetForAST = &lowered.irDecls;
             visitor.EmitDeclsInContainer(lowered.program);
         }