IR: next phase of "everything is an instruction" (#433)

The main practical change here is that things that used to be `IRValue`s, like literals, are now being expressed as instructions in the global scope.

In order to validate that things are actually being handled correctly, this change introduces an explicit "validation" pass that can be run on the IR to check for different invariants (although it doesn't check many of the important ones right now). I've left the validation pass turned off by default, but with a command-line flag to enable it. We may want to make it be on by default in debug builds, just to keep us honest. The main invariant for the moment is that when on IR instruction is used as an operand to another, it had better come from the same IR module.

Some of the existing passes were violating this rule, in particular when it came to cloning of witness tables related to global generic parameter substitution. Those features can in theory be handled better now by allowing `specialize` instructions at other scopes, but I didn't want to over-complicate this change, so I make just enough fixes to ensure that these steps always clone witness tables they get from the "symbols" on an IR specialization context. In order for this to work when recursively specializing, I had to ensure that the logic for generic specialization had a notion of a "parent" specialization context that it would fall back to to perform cloning when necessary.

This change keeps the logic that was caching and re-using the instructions for literal values within a module, but adds some logic that isn't really being tested right now for picking the right parent instruction to insert a constant instruction into. This logic doesn't trigger right now because all of the cases we are using it on have zero operands (and so they always get "hoisted" to the global scope), but eventually for things like types we want to be able to support instructions with operands (e.g., `vector<float, 4>`) and handle the case where some of those operands come from different scopes (e.g., when nested inside a generic).

The final change here is mostly cosmetic: the `IRBuilder` is now more abstract about where insertion occurs: it tracks a single `IRParentInst` to insert into, and then an optional `IRInst` to insert before. In the common case, that parent is an `IRBlock`, but it could conceivably also be the global scope, or a witness table, etc. Use sites where we used to change those fields directly now use distinct methods `setInsertInto(parent)` and `setInsertBefore(inst)` which capture the two cases we care about. Accessors are also defined to extract the current block (if the current parent is a block), and the current "function" (global value with code, if the current parent is a global value with code, or a block inside one).

With this work in place, it should be possible for a follow-on change to start putting `specialize` instructions at the global scope and thus clean up some of the on-the-fly specialization work. This work should also help with some of the requirements around a distinct IR-level type system and more explicit generics.

1 files changed, 26 insertions, 18 deletions

diff --git a/source/slang/lower-to-ir.cpp b/source/slang/lower-to-ir.cpp
index 9508bb86d..cfd1c89a4 100644
--- a/source/slang/lower-to-ir.cpp
+++ b/source/slang/lower-to-ir.cpp
@@ -7,6 +7,7 @@
 #include "ir-constexpr.h"
 #include "ir-insts.h"
 #include "ir-ssa.h"
+#include "ir-validate.h"
 #include "mangle.h"
 #include "type-layout.h"
 #include "visitor.h"
@@ -1931,8 +1932,8 @@ struct StmtLoweringVisitor : StmtVisitor<StmtLoweringVisitor>
     {
         auto builder = getBuilder();
 
-        auto prevBlock = builder->curBlock;
-        auto parentFunc = prevBlock ? prevBlock->getParent() : builder->curFunc;
+        auto prevBlock = builder->getBlock();
+        auto parentFunc = prevBlock ? prevBlock->getParent() : builder->getFunc();
 
         // If the previous block doesn't already have
         // a terminator instruction, then be sure to
@@ -1944,8 +1945,7 @@ struct StmtLoweringVisitor : StmtVisitor<StmtLoweringVisitor>
 
         parentFunc->addBlock(block);
 
-        builder->curFunc = parentFunc;
-        builder->curBlock = block;
+        builder->setInsertInto(block);
     }
 
     // Start a new block at the current location.
@@ -2495,7 +2495,7 @@ struct StmtLoweringVisitor : StmtVisitor<StmtLoweringVisitor>
 
         // Remember the initial block so that we can add to it
         // after we've collected all the `case`s
-        auto initialBlock = builder->curBlock;
+        auto initialBlock = builder->getBlock();
 
         // Next, create a block to use as the target for any `break` statements
         auto breakLabel = createBlock();
@@ -2504,8 +2504,7 @@ struct StmtLoweringVisitor : StmtVisitor<StmtLoweringVisitor>
         // that we can find it for nested statements.
         context->shared->breakLabels.Add(stmt, breakLabel);
 
-        builder->curFunc = initialBlock->getParent();
-        builder->curBlock = nullptr;
+        builder->setInsertInto(initialBlock->getParent());
 
         // Iterate over the body of the statement, looking
         // for `case` or `default` statements:
@@ -2525,10 +2524,11 @@ struct StmtLoweringVisitor : StmtVisitor<StmtLoweringVisitor>
         // Double check that we aren't in the initial
         // block, so we don't get tripped up on an
         // empty `switch`.
-        if(builder->curBlock != initialBlock)
+        auto curBlock = builder->getBlock();
+        if(curBlock != initialBlock)
         {
             // Is the block already terminated?
-            if(!builder->curBlock->getTerminator())
+            if(!curBlock->getTerminator())
             {
                 // Not terminated, so add one.
                 builder->emitBreak(breakLabel);
@@ -2542,7 +2542,7 @@ struct StmtLoweringVisitor : StmtVisitor<StmtLoweringVisitor>
         // Now that we've collected the cases, we are
         // prepared to emit the `switch` instruction
         // itself.
-        builder->curBlock = initialBlock;
+        builder->setInsertInto(initialBlock);
         builder->emitSwitch(
             conditionVal,
             breakLabel,
@@ -2819,9 +2819,13 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
                     context->irBuilder->createWitnessTableEntry(witnessTable,
                         context->irBuilder->getDeclRefVal(subInheritanceDeclRef), cpyTable);
 
-                    // HACK: we are re-using the entries in a pre-existing table here,
-                    // which is not how things are supposed to work.
-                    cpyTable->children = witnessTable->children;
+                    // We need to copy all the entries from the original table to this new table.
+                    for (auto entry : witnessTable->getEntries())
+                    {
+                        context->irBuilder->createWitnessTableEntry(cpyTable,
+                            entry->requirementKey.get(),
+                            entry->satisfyingVal.get());
+                    }
 
                     witnessTablesDictionary.Add(cpyTable->mangledName, cpyTable);
                     walkInheritanceHierarchyAndCreateWitnessTableCopies(witnessTable, subType, subInheritanceDeclRef.getDecl());
@@ -3024,7 +3028,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
             IRBuilder subBuilderStorage = *getBuilder();
             IRBuilder* subBuilder = &subBuilderStorage;
 
-            subBuilder->curFunc = irGlobal;
+            subBuilder->setInsertInto(irGlobal);
 
             IRGenContext subContextStorage = *context;
             IRGenContext* subContext = &subContextStorage;
@@ -3034,7 +3038,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
             // TODO: set up a parent IR decl to put the instructions into
 
             IRBlock* entryBlock = subBuilder->emitBlock();
-            subBuilder->curBlock = entryBlock;
+            subBuilder->setInsertInto(entryBlock);
 
             LoweredValInfo initVal = lowerLValueExpr(subContext, initExpr);
             subContext->irBuilder->emitReturn(getSimpleVal(subContext, initVal));
@@ -3570,7 +3574,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
         // need to create an IR function here
 
         IRFunc* irFunc = subBuilder->createFunc();
-        subBuilder->curFunc = irFunc;
+        subBuilder->setInsertInto(irFunc);
 
         trySetMangledName(irFunc, decl);
 
@@ -3673,7 +3677,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
             // This is a function definition, so we need to actually
             // construct IR for the body...
             IRBlock* entryBlock = subBuilder->emitBlock();
-            subBuilder->curBlock = entryBlock;
+            subBuilder->setInsertInto(entryBlock);
 
             UInt paramTypeIndex = 0;
             for( auto paramInfo : parameterLists.params )
@@ -3790,7 +3794,7 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
 
             // We need to carefully add a terminator instruction to the end
             // of the body, in case the user didn't do so.
-            if (!subContext->irBuilder->curBlock->getTerminator())
+            if (!subContext->irBuilder->getBlock()->getTerminator())
             {
                 if (irResultType->Equals(context->getSession()->getVoidType()))
                 {
@@ -4227,6 +4231,8 @@ IRModule* generateIRForTranslationUnit(
         ensureDecl(context, decl);
     }
 
+    validateIRModuleIfEnabled(compileRequest, module);
+
     // We will perform certain "mandatory" optimization passes now.
     // These passes serve two purposes:
     //
@@ -4267,6 +4273,8 @@ IRModule* generateIRForTranslationUnit(
     // "fragile" in that we'd now need to recompile when
     // a module we depend on changes.
 
+    validateIRModuleIfEnabled(compileRequest, module);
+
     // If we are being sked to dump IR during compilation,
     // then we can dump the initial IR for the module here.
     if(compileRequest->shouldDumpIR)