Fix optimization pass not converging. (#2725)

* Fix optimization pass not converging.

* Fix.

* Fix tests.

---------

Co-authored-by: Yong He <yhe@nvidia.com>

7 files changed, 64 insertions, 60 deletions

diff --git a/source/slang/slang-ir-propagate-func-properties.cpp b/source/slang/slang-ir-propagate-func-properties.cpp
index f98a77fc7..5b673e02a 100644
--- a/source/slang/slang-ir-propagate-func-properties.cpp
+++ b/source/slang/slang-ir-propagate-func-properties.cpp
@@ -117,7 +117,8 @@ bool propagateFuncProperties(IRModule* module)
                         default:
                             // We have a inst that has side effect and is not understood by this method.
                             // e.g. bufferStore, discard, etc.
-                            return true;
+                            hasSideEffectCall = true;
+                            break;
                         }
                     }
 
diff --git a/source/slang/slang-ir-redundancy-removal.cpp b/source/slang/slang-ir-redundancy-removal.cpp
index 32b349a4f..dd92630b3 100644
--- a/source/slang/slang-ir-redundancy-removal.cpp
+++ b/source/slang/slang-ir-redundancy-removal.cpp
@@ -150,6 +150,7 @@ struct RedundancyRemovalContext
             if (resultInst != instP)
             {
                 instP->replaceUsesWith(resultInst);
+                instP->removeAndDeallocate();
                 result = true;
             }
             else if (isMovableInst(resultInst))
@@ -182,7 +183,6 @@ bool removeRedundancy(IRModule* module)
         if (auto func = as<IRFunc>(inst))
         {
             changed |= removeRedundancyInFunc(func);
-            changed |= eliminateRedundantLoadStore(func);
         }
     }
     return changed;
@@ -197,7 +197,12 @@ bool removeRedundancyInFunc(IRGlobalValueWithCode* func)
     RedundancyRemovalContext context;
     context.dom = computeDominatorTree(func);
     DeduplicateContext deduplicateCtx;
-    return context.removeRedundancyInBlock(deduplicateCtx, func, root);
+    bool result = context.removeRedundancyInBlock(deduplicateCtx, func, root);
+    if (auto normalFunc = as<IRFunc>(func))
+    {
+        result |= eliminateRedundantLoadStore(normalFunc);
+    }
+    return result;
 }
 
 static IRInst* _getRootVar(IRInst* inst)
diff --git a/source/slang/slang-ir-sccp.cpp b/source/slang/slang-ir-sccp.cpp
index 691bd7ff0..4e589c1fe 100644
--- a/source/slang/slang-ir-sccp.cpp
+++ b/source/slang/slang-ir-sccp.cpp
@@ -1675,6 +1675,17 @@ bool applySparseConditionalConstantPropagation(
     return changed;
 }
 
+bool applySparseConditionalConstantPropagationForGlobalScope(
+    IRModule* module)
+{
+    SharedSCCPContext shared;
+    shared.module = module;
+    SCCPContext globalContext;
+    globalContext.shared = &shared;
+    globalContext.code = nullptr;
+    bool changed = globalContext.applyOnGlobalScope(module);
+    return changed;
+}
 
 bool applySparseConditionalConstantPropagation(IRInst* func)
 {
diff --git a/source/slang/slang-ir-sccp.h b/source/slang/slang-ir-sccp.h
index f97cbb366..7b83ce10f 100644
--- a/source/slang/slang-ir-sccp.h
+++ b/source/slang/slang-ir-sccp.h
@@ -16,6 +16,8 @@ namespace Slang
         /// Returns true if IR is changed.
     bool applySparseConditionalConstantPropagation(
         IRModule*       module);
+    bool applySparseConditionalConstantPropagationForGlobalScope(
+        IRModule* module);
 
     bool applySparseConditionalConstantPropagation(IRInst* func);
 
diff --git a/source/slang/slang-ir-simplify-cfg.cpp b/source/slang/slang-ir-simplify-cfg.cpp
index e98d14a0c..4d5b6e21b 100644
--- a/source/slang/slang-ir-simplify-cfg.cpp
+++ b/source/slang/slang-ir-simplify-cfg.cpp
@@ -315,42 +315,6 @@ static bool isTrivialIfElse(IRIfElse* condBranch, bool& isTrueBranchTrivial, boo
     return false;
 }
 
-#if 0
-static bool tryMoveFalseBranchToTrueBranch(IRBuilder& builder, IRIfElse* ifElseInst)
-{
-    auto falseBlock = ifElseInst->getFalseBlock();
-    if (falseBlock == ifElseInst->getAfterBlock())
-        return false;
-    if (auto termInst = as<IRUnconditionalBranch>(falseBlock->getTerminator()))
-    {
-        // We can't fold a branch with arguments into the ifElse.
-        if (termInst->getArgCount() != 0)
-            return false;
-    }
-    ifElseInst->trueBlock.set(falseBlock);
-    ifElseInst->falseBlock.set(ifElseInst->getAfterBlock());
-    builder.setInsertBefore(ifElseInst);
-    auto newCondition = builder.emitNot(builder.getBoolType(), ifElseInst->getCondition());
-    ifElseInst->condition.set(newCondition);
-    return true;
-}
-#endif
-
-static bool tryEliminateFalseBranch(IRIfElse* ifElseInst)
-{
-    auto falseBlock = ifElseInst->getFalseBlock();
-    if (falseBlock == ifElseInst->getAfterBlock())
-        return false;
-    if (auto termInst = as<IRUnconditionalBranch>(falseBlock->getTerminator()))
-    {
-        // We can't fold a branch with arguments into the ifElse.
-        if (termInst->getArgCount() != 0)
-            return false;
-    }
-    ifElseInst->falseBlock.set(ifElseInst->getAfterBlock());
-    return true;
-}
-
 static bool trySimplifyIfElse(IRBuilder& builder, IRIfElse* ifElseInst)
 {
     bool isTrueBranchTrivial = false;
@@ -371,18 +335,6 @@ static bool trySimplifyIfElse(IRBuilder& builder, IRIfElse* ifElseInst)
             return true;
         }
     }
-    else if (isTrueBranchTrivial)
-    {
-        // If true branch is empty, we move false branch to true branch and invert the condition.
-        // TODO: diabled for now since our auto-diff pass can't handle loops whose body is on the false
-        // side of condition.
-        //return tryMoveFalseBranchToTrueBranch(builder, ifElseInst);
-    }
-    else if (isFalseBranchTrivial)
-    {
-        // If false branch is empty, we set it to afterBlock.
-        return tryEliminateFalseBranch(ifElseInst);
-    }
     return false;
 }
 
diff --git a/source/slang/slang-ir-ssa-simplification.cpp b/source/slang/slang-ir-ssa-simplification.cpp
index a0acf5082..e6c6e353f 100644
--- a/source/slang/slang-ir-ssa-simplification.cpp
+++ b/source/slang/slang-ir-ssa-simplification.cpp
@@ -20,26 +20,45 @@ namespace Slang
     {
         bool changed = true;
         const int kMaxIterations = 8;
+        const int kMaxFuncIterations = 16;
         int iterationCounter = 0;
+
         while (changed && iterationCounter < kMaxIterations)
         {
             changed = false;
             changed |= hoistConstants(module);
             changed |= deduplicateGenericChildren(module);
-            changed |= applySparseConditionalConstantPropagation(module);
+            changed |= propagateFuncProperties(module);
+            changed |= removeUnusedGenericParam(module);
+            changed |= applySparseConditionalConstantPropagationForGlobalScope(module);
             changed |= peepholeOptimize(module);
-            changed |= removeRedundancy(module);
-            changed |= simplifyCFG(module);
+
+            for (auto inst : module->getGlobalInsts())
+            {
+                auto func = as<IRGlobalValueWithCode>(inst);
+                if (!func)
+                    continue;
+                bool funcChanged = true;
+                int funcIterationCount = 0;
+                while (funcChanged && funcIterationCount < kMaxFuncIterations)
+                {
+                    funcChanged = false;
+                    funcChanged |= applySparseConditionalConstantPropagation(func);
+                    funcChanged |= peepholeOptimize(func);
+                    funcChanged |= removeRedundancyInFunc(func);
+                    funcChanged |= simplifyCFG(func);
+                    eliminateDeadCode(func);
+                    funcChanged |= constructSSA(func);
+                    changed |= funcChanged;
+                    funcIterationCount++;
+                }
+            }
 
             // Note: we disregard the `changed` state from dead code elimination pass since
             // SCCP pass could be generating temporarily evaluated constant values and never actually use them.
             // DCE will always remove those nearly generated consts and always returns true here.
             eliminateDeadCode(module);
 
-            changed |= propagateFuncProperties(module);
-
-            changed |= constructSSA(module);
-            changed |= removeUnusedGenericParam(module);
             iterationCounter++;
         }
     }
diff --git a/source/slang/slang-ir.cpp b/source/slang/slang-ir.cpp
index 206d73e3f..69870c128 100644
--- a/source/slang/slang-ir.cpp
+++ b/source/slang/slang-ir.cpp
@@ -7093,8 +7093,6 @@ namespace Slang
         case kIROp_Add:
         case kIROp_Sub:
         case kIROp_Mul:
-        //case kIROp_Div:   // TODO: We could split out integer vs. floating-point div/mod and assume the floating-point cases have no side effects
-        //case kIROp_Rem:
         case kIROp_Lsh:
         case kIROp_Rsh:
         case kIROp_Eql:
@@ -7138,6 +7136,22 @@ namespace Slang
         case kIROp_BackwardDifferentiatePropagate:
         case kIROp_DetachDerivative:
             return false;
+
+        case kIROp_Div:
+        case kIROp_IRem:
+            if (isIntegralScalarOrCompositeType(getFullType()))
+            {
+                if (auto intLit = as<IRIntLit>(getOperand(1)))
+                {
+                    if (intLit->getValue() != 0)
+                        return false;
+                }
+                return true;
+            }
+            return false;
+
+        case kIROp_FRem:
+            return false;
         }
         return true;
     }