Falcor fixes (#402)

* Re-define deprecated compile flags

By including these flags in the header file, with a value of zero, we can allow some existing code to compile even after the major changes to the implementation.

* The `SLANG_COMPILE_FLAG_NO_CHECKING` option will effectively be ignored, since checking is always enabled.

* The `SLANG_COMPILE_FLAG_SPLIT_MIXED_TYPES` option will now act as if it is always enabled (and indeed some of the code has been relying on this flag being set always).

* Make subscript operators writable for writable textures

This even had a `TODO` comment saying that we needed to fix it, and now I'm seeing semantic checking failures because we didn't define these and so we find assignment to non l-values.

* Fix definitions of any() and all() intrinsics

These should always return a scalar `bool` value, but they were being defined wrong in two ways:

1. They were using their generic type parameter `T` in the return type
2. They were returning a vector in the vector case, and a matrix in the matrix case.

This change just alters the return type to be `bool` in all cases.

* Fix bug in SSA construction

When eliminating a trivial phi node, it is possible that the phi is still recorded as the "latest" value for a local variable in its block.
When later code queries that value from the block (which can happen whenever another block looks up a variable in its predecessors), it would get the old phi and not the replacement value.

I simply added a loop that checks if the value we look up is a phi that got replaced, and then continues with the replacement value (which might itself be a phi...). A more advanced solution might try to get clever and have the map itself hold `IRUse` values so that we can replace them seamlessly.

* Simplify IR control flow representation

This change gets rid of various special-case operations for conditional and unconditional branches, and instead requires emit logic to recognize when a direct branch is targetting a `break` or `continue` label.

The new approach here isn't perfect, but it seems beter than what we had before, because it can actually work in the presence of control-flow optimizations (including our current critical-edge-splitting step).

* Load from groupshared isn't groupshared

When loading from a `groupshared` variable, the resulting temporary shouldn't have the `groupshared` qualifier on it.
This might eventually need to generalize to a better understanding of storage modifiers in the IR, but I don't really want to deal with that right now.

* Don't emit references to typedefs in output code

Now that we are using the IR for all codegen, we shouldn't be dealing with surface-level things like `typedef` declarations in the output code; just use the type that was being referred to in the first place.

* Fix floating-point literal printing for IR

The IR was calling `emit()` instead of `Emit()` (we really need to normalize our convention here), and was implicitly invoking a default constructor on `String` that takes a `double` (that constructor should really be marked `explicit`), and which doesn't meet our requirements for printing floating-point values.

* Fix error when importing module that doesn't parse

We already added a case to bail out if semantic checking fails, but neglected to add a case if there is an error during parsing of a module to be imported.

Note: this logic doesn't correctly register the module as being loaded (but still in error), so users could see multiple error messages if there are multiple `import`s for the same module.

* Improve error message for overload resolution failure

- Drop debugging info from the candidate printing

- Add cases to print `double` and `half` types properly

* Fixup: switch loopTest to ifElse in expected IR output

1 files changed, 144 insertions, 106 deletions

diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp
index eaabd520c..0a71c7fa9 100644
--- a/source/slang/emit.cpp
+++ b/source/slang/emit.cpp
@@ -109,8 +109,6 @@ struct SharedEmitContext
 
     HashSet<String> irDeclsVisited;
 
-    Dictionary<IRBlock*, IRBlock*> irMapContinueTargetToLoopHead;
-
     HashSet<String> irTupleTypes;
 };
 
@@ -1095,17 +1093,14 @@ struct EmitVisitor
     UNEXPECTED(PtrType);
 
 #undef UNEXPECTED
+
     void visitNamedExpressionType(NamedExpressionType* type, TypeEmitArg const& arg)
     {
-        // Named types are valid for GLSL
-        if (context->shared->target == CodeGenTarget::GLSL)
-        {
-            emitTypeImpl(GetType(type->declRef), arg.declarator);
-            return;
-        }
+        // We will always emit the actual type referenced by
+        // a named type declaration, rather than try to produce
+        // equivalent `typedef` declarations in the output.
 
-        EmitDeclRef(type->declRef);
-        EmitDeclarator(arg.declarator);
+        emitTypeImpl(GetType(type->declRef), arg.declarator);
     }
 
     void visitBasicExpressionType(BasicExpressionType* basicType, TypeEmitArg const& arg)
@@ -4808,7 +4803,7 @@ emitDeclImpl(decl, nullptr);
             break;
 
         case kIROp_FloatLit:
-            emit(((IRConstant*) inst)->u.floatVal);
+            Emit(((IRConstant*) inst)->u.floatVal);
             break;
 
         case kIROp_boolConst:
@@ -6241,6 +6236,21 @@ emitDeclImpl(decl, nullptr);
         }
     }
 
+    enum LabelOp
+    {
+        kLabelOp_break,
+        kLabelOp_continue,
+
+        kLabelOpCount,
+    };
+
+    struct LabelStack
+    {
+        LabelStack* parent;
+        IRBlock*    block;
+        LabelOp     op;
+    };
+
     // We want to emit a range of code in the IR, represented
     // by the blocks that are logically in the interval [begin, end)
     // which we consider as a single-entry multiple-exit region.
@@ -6253,11 +6263,69 @@ emitDeclImpl(decl, nullptr);
     void emitIRStmtsForBlocks(
         EmitContext*    ctx,
         IRBlock*        begin,
-        IRBlock*        end)
+        IRBlock*        end,
+
+        // Labels to use at the start
+        LabelStack*     initialLabels,
+
+        // Labels to switch to after emitting first basic block
+        LabelStack*     labels = nullptr)
     {
+        if(!labels)
+            labels = initialLabels;
+
+        auto useLabels = initialLabels;
+
         IRBlock* block = begin;
         while(block != end)
         {
+            // If the block we are trying to emit has been registered as a
+            // destination label (e.g. for a loop or `switch`) then we
+            // may need to emit a `break` or `continue` as needed.
+            //
+            // TODO: we eventually need to handle the possibility of
+            // multi-level break/continue targets, which could be challenging.
+
+            // First, figure out which block has been registered as
+            // the current `break` and `continue` target.
+            IRBlock* registeredBlock[kLabelOpCount] = {};
+            for( auto ll = labels; ll; ll = ll->parent )
+            {
+                if(!registeredBlock[ll->op])
+                {
+                    registeredBlock[ll->op] = ll->block;
+                }
+            }
+
+            // Next, search in the active labels we are allowed to use,
+            // and see if the block we are trying to branch to is an
+            // available break/continue target.
+            for(auto ll = useLabels; ll; ll = ll->parent)
+            {
+                if(ll->block == block)
+                {
+                    // We are trying to go to a block that has been regsitered as a label.
+
+                    if(block != registeredBlock[ll->op])
+                    {
+                        // ERROR: need support for multi-level break/continue to pull this one off!
+                    }
+
+                    switch(ll->op)
+                    {
+                    case kLabelOp_break:
+                        emit("break;\n");
+                        break;
+
+                    case kLabelOp_continue:
+                        emit("continue;\n");
+                        break;
+                    }
+
+                    return;
+                }
+            }
+
             // Start by emitting the non-terminator instructions in the block.
             auto terminator = block->getLastInst();
             assert(isTerminatorInst(terminator));
@@ -6285,28 +6353,6 @@ emitDeclImpl(decl, nullptr);
                 emitIRInst(ctx, terminator);
                 return;
 
-            case kIROp_if:
-                {
-                    // One-sided `if` statement
-                    auto t = (IRIf*)terminator;
-
-                    auto trueBlock = t->getTrueBlock();
-                    auto afterBlock = t->getAfterBlock();
-
-                    emit("if(");
-                    emitIROperand(ctx, t->getCondition());
-                    emit(")\n{\n");
-                    emitIRStmtsForBlocks(
-                        ctx,
-                        trueBlock,
-                        afterBlock);
-                    emit("}\n");
-
-                    // Continue with the block after the `if`
-                    block = afterBlock;
-                }
-                break;
-
             case kIROp_ifElse:
                 {
                     // Two-sided `if` statement
@@ -6316,19 +6362,31 @@ emitDeclImpl(decl, nullptr);
                     auto falseBlock = t->getFalseBlock();
                     auto afterBlock = t->getAfterBlock();
 
+                    // TODO: consider simplifying the code in
+                    // the case where `trueBlock == afterBlock`
+                    // so that we output `if(!cond) { falseBlock }`
+                    // instead of the current `if(cond) {} else {falseBlock}`
+
                     emit("if(");
                     emitIROperand(ctx, t->getCondition());
                     emit(")\n{\n");
                     emitIRStmtsForBlocks(
                         ctx,
                         trueBlock,
-                        afterBlock);
-                    emit("}\nelse\n{\n");
-                    emitIRStmtsForBlocks(
-                        ctx,
-                        falseBlock,
-                        afterBlock);
+                        afterBlock,
+                        labels);
                     emit("}\n");
+                    // Don't emit the false block if it would be empty
+                    if(falseBlock != afterBlock)
+                    {
+                        emit("else\n{\n");
+                        emitIRStmtsForBlocks(
+                            ctx,
+                            falseBlock,
+                            afterBlock,
+                            labels);
+                        emit("}\n");
+                    }
 
                     // Continue with the block after the `if`
                     block = afterBlock;
@@ -6342,7 +6400,6 @@ emitDeclImpl(decl, nullptr);
 
                     auto targetBlock = t->getTargetBlock();
                     auto breakBlock = t->getBreakBlock();
-                    auto continueBlock = t->getContinueBlock();
 
                     UInt argCount = t->getArgCount();
                     static const UInt kFixedArgCount = 3;
@@ -6352,6 +6409,25 @@ emitDeclImpl(decl, nullptr);
                         t->getArgs() + kFixedArgCount,
                         targetBlock);
 
+                    // Set up entries on our label stack for break/continue
+
+                    LabelStack subBreakLabel;
+                    subBreakLabel.parent = labels;
+                    subBreakLabel.block = breakBlock;
+                    subBreakLabel.op = kLabelOp_break;
+
+                    // Note: when forming the `continue` label, we don't
+                    // actually point at the "continue block" from the loop
+                    // statement, because we aren't actually going to
+                    // generate an ordinary continue caluse in a `for` loop.
+                    //
+                    // Instead, our `continue` label will always be the
+                    // loop header.
+                    LabelStack subContinueLabel;
+                    subContinueLabel.parent = &subBreakLabel;
+                    subContinueLabel.block = targetBlock;
+                    subContinueLabel.op = kLabelOp_continue;
+
                     if (auto loopControlDecoration = t->findDecoration<IRLoopControlDecoration>())
                     {
                         switch (loopControlDecoration->mode)
@@ -6365,75 +6441,25 @@ emitDeclImpl(decl, nullptr);
                         }
                     }
 
-                    // The challenging case for a loop is when
-                    // there is a `continue` block that we
-                    // need to deal with.
-                    //
-                    if (continueBlock == targetBlock)
-                    {
-                        // There is no continue block, so
-                        // we only need to emit an endless
-                        // loop and then manually `break`
-                        // out of it in the right place(s)
-                        emit("for(;;)\n{\n");
+                    emit("for(;;)\n{\n");
 
-                        emitIRStmtsForBlocks(
-                            ctx,
-                            targetBlock,
-                            nullptr);
-
-                        emit("}\n");
-                    }
-                    else
-                    {
-                        // Okay, we've got a `continue` block,
-                        // which means we really want to emit
-                        // something akin to:
-                        //
-                        //     for(;; <continueBlock>) { <bodyBlock> }
-                        //
-                        // In principle this isn't so bad, since the
-                        // first case is just interVal [`continueBlock`, `targetBlock`)
-                        // and the latter is the interval [`targetBlock`, `continueBlock`).
-                        //
-                        // The challenge of course is that a `for` statement
-                        // only supports *expressions* in the continue part,
-                        // and we might have expanded things into multiple
-                        // instructions (especially if we inlined or desugared anything).
-                        //
-                        // There are a variety of ways we can support lowering this,
-                        // but for now we are going to do something expedient
-                        // that mimics what `fxc` seems to do:
-                        //
-                        // - Output loop body as `for(;;) { <bodyBlock> <continueBlock> }`
-                        // - At any `continue` site, output `{ <continueBlock>; continue; }`
-                        //
-                        // This isn't ideal because it leads to code duplication, but
-                        // it matches what `fxc` does so hopefully it will be the
-                        // best option for our tests.
-                        //
-
-                        emit("for(;;)\n{\n");
-
-                        // Register information so that `continue` sites
-                        // can do the right thing:
-                        ctx->shared->irMapContinueTargetToLoopHead.Add(continueBlock, targetBlock);
-
-
-                        emitIRStmtsForBlocks(
-                            ctx,
-                            targetBlock,
-                            nullptr);
-
-                        emit("}\n");
+                    emitIRStmtsForBlocks(
+                        ctx,
+                        targetBlock,
+                        nullptr,
+                        // For the first block, we only want the `break` label active
+                        &subBreakLabel,
+                        // After the first block, we can safely use the `continue` label too
+                        &subContinueLabel);
 
-                    }
+                    emit("}\n");
 
                     // Continue with the block after the loop
                     block = breakBlock;
                 }
                 break;
 
+#if 0
             case kIROp_break:
                 {
                     auto t = (IRBreak*)terminator;
@@ -6506,6 +6532,7 @@ emitDeclImpl(decl, nullptr);
                     block = afterBlock;
                 }
                 break;
+#endif
 
             case kIROp_unconditionalBranch:
                 {
@@ -6561,6 +6588,12 @@ emitDeclImpl(decl, nullptr);
                     auto breakLabel = t->getBreakLabel();
                     auto defaultLabel = t->getDefaultLabel();
 
+                    // Register the block to be used for our `break` target
+                    LabelStack subLabels;
+                    subLabels.parent = labels;
+                    subLabels.op = kLabelOp_break;
+                    subLabels.block = breakLabel;
+
                     // We need to track whether we've dealt with
                     // the `default` case already.
                     bool defaultLabelHandled = false;
@@ -6645,7 +6678,7 @@ emitDeclImpl(decl, nullptr);
 
                         // Now emit the statements for this case.
                         emit("{\n");
-                        emitIRStmtsForBlocks(ctx, caseLabel, caseEndLabel);
+                        emitIRStmtsForBlocks(ctx, caseLabel, caseEndLabel, &subLabels);
                         emit("}\n");
                     }
 
@@ -6656,7 +6689,7 @@ emitDeclImpl(decl, nullptr);
                     {
                         emit("default:\n");
                         emit("{\n");
-                        emitIRStmtsForBlocks(ctx, defaultLabel, breakLabel);
+                        emitIRStmtsForBlocks(ctx, defaultLabel, breakLabel, &subLabels);
                         emit("break;\n");
                         emit("}\n");
                     }
@@ -6668,6 +6701,11 @@ emitDeclImpl(decl, nullptr);
                 break;
             }
 
+            // After we've emitted the first block, we are safe from accidental
+            // cases where we'd emit an entire loop body as a single `continue`,
+            // so we can safely switch in whatever labels are intended to be used.
+            useLabels = labels;
+
             // If we reach this point, then we've emitted
             // one block, and we have a new block where
             // control flow continues.
@@ -6928,7 +6966,7 @@ emitDeclImpl(decl, nullptr);
 
             // Need to emit the operations in the blocks of the function
 
-            emitIRStmtsForBlocks(ctx, func->getFirstBlock(), nullptr);
+            emitIRStmtsForBlocks(ctx, func->getFirstBlock(), nullptr, nullptr);
 
             emit("}\n");
         }
@@ -7612,7 +7650,7 @@ emitDeclImpl(decl, nullptr);
             initFuncName.append("_init");
             emitIRType(ctx, varType, initFuncName);
             Emit("()\n{\n");
-            emitIRStmtsForBlocks(ctx, varDecl->firstBlock, nullptr);
+            emitIRStmtsForBlocks(ctx, varDecl->firstBlock, nullptr, nullptr);
             Emit("}\n");
         }