Fix the order changing issue

Author: jkwak-work

source/slang/slang-ir-specialize.cpp

@@ -3121,61 +3121,7 @@ IRInst* specializeGenericImpl(
         // instructions only, because parameters were dealt
         // with explictly at an earlier point.
         //
-        IRInstList<IRInst> ordinaryInsts = bb->getOrdinaryInsts();
-
-        // After IRWitnessTable became Hoistable, they are removed and insered back by
-        // `addHoistableInst()`. But when they are re-inserted, the order it appears in the block is
-        // changed. When IRWitnessTable refers to IRSpecialize, as an example, IRSpecialize must be
-        // cloned before the cloning of IRWitnessTable. It is because the operands are assumed to be
-        // cloned before the cloning of IRInst.
-        //
-        // We need to resolve the dependency problem by changing the order of them.
-        //
-        List<IRInst*> insts;
-        int instCount = 0;
-        for (IRInst* ii : ordinaryInsts)
-        {
-            SLANG_UNUSED(ii);
-            instCount++;
-        }
-        insts.reserve(instCount);
-
-        // First, add all instructions in their original order
-        for (IRInst* ii : ordinaryInsts)
-        {
-            insts.add(ii);
-        }
-
-        // We'll need multiple passes to handle complex dependency chains
-        bool madeChanges = true;
-        while (madeChanges)
-        {
-            madeChanges = false;
-
-            for (int i = 0; i < instCount; i++)
-            {
-                IRInst* current = insts[i];
-
-                // Find all instructions that `current` depends on
-                for (int j = i + 1; j < instCount; j++)
-                {
-                    IRInst* later = insts[j];
-
-                    if (IsIRInstDependOn(current, later))
-                    {
-                        // `current` depends on `later`, so `later` must come before `current`.
-                        insts.removeAt(j);
-                        insts.insert(i, later);
-
-                        // mark the change, and continue to the next.
-                        madeChanges = true;
-                        i++;
-                    }
-                }
-            }
-        }
-
-        for (auto ii : insts)
+        for (auto ii : bb->getOrdinaryInsts())
         {
             // The last block of the generic is expected to end with
             // a `return` instruction for the specialized value that

source/slang/slang-ir.cpp

@@ -1694,6 +1694,30 @@ void addHoistableInst(IRBuilder* builder, IRInst* inst)
     while (insertBeforeInst && insertBeforeInst->getOp() == kIROp_Param)
         insertBeforeInst = insertBeforeInst->getNextInst();
 
+    if (inst->getOp() == kIROp_WitnessTable)
+    {
+        // WitnessTable can refer to IRSpecialize from its WitnessTableEntry
+        // children. In this case, specialize insts must be cloned before the
+        // WitnessTable. Similar an WitnessTables can have depdency to another
+        // WitnessTable.
+        //
+        for (IRInst* iter = insertBeforeInst; iter;)
+        {
+            bool mayHaveDependency = false;
+            switch (iter->getOp())
+            {
+            case kIROp_Specialize:
+            case kIROp_WitnessTable:
+                mayHaveDependency = true;
+                break;
+            }
+
+            iter = iter->getNextInst();
+            if (mayHaveDependency)
+                insertBeforeInst = iter;
+        }
+    }
+
     // For instructions that will be placed at module scope,
     // we don't care about relative ordering, but for everything
     // else, we want to ensure that an instruction comes after
@@ -4609,6 +4633,16 @@ void addGlobalValue(IRBuilder* builder, IRInst* value)
         parent = builder->getModule()->getModuleInst();
     }
 
+    // If the value is already in the parent, keep it as it. Because WitnessTable is Hoistable, the
+    // parent can have only one instance of this WitnessTable. The order among siblings should
+    // remain because the later siblings may have dependency to the earlier siblings.
+    // 
+    if (parent == value->parent)
+    {
+        SLANG_ASSERT(getIROpInfo(kIROp_WitnessTable).isHoistable());
+        return;
+    }
+
     // If it turns out that we are inserting into the
     // current "insert into" parent for the builder, then
     // we need to respect its "insert before" setting

source/slang/slang-lower-to-ir.cpp

@@ -8040,9 +8040,9 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
                                 astReqWitnessTable,
                                 irSatisfyingWitnessTable,
                                 mapASTToIRWitnessTable);
-                        }
 
-                        irSatisfyingWitnessTable->moveToEnd();
+                            irSatisfyingWitnessTable->moveToEnd();
+                        }
                     }
                     irSatisfyingVal = irSatisfyingWitnessTable;
                 }
@@ -8214,9 +8214,9 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
                     irWitnessTable,
                     mapASTToIRWitnessTable);
             }
-        }
 
-        irWitnessTable->moveToEnd();
+            irWitnessTable->moveToEnd();
+        }
 
         return LoweredValInfo::simple(
             finishOuterGenerics(subBuilder, irWitnessTable, outerGeneric));