[MERGE #5255 @meg-gupta] OS#17677370 : Avoid allocating local value table before legality check

Merge pull request #5255 from meg-gupta:fixexcessmem

Author: Meghana Gupta

lib/Backend/FlowGraph.cpp

@@ -4610,13 +4610,9 @@ void
 BasicBlock::CheckLegalityAndFoldPathDepBranches(GlobOpt* globOpt)
 {
     IR::LabelInstr * lastBranchTarget = nullptr;
-
-    // For the block we start processing from, we maintain a valueTable, we go over the instrs in the block
-    // If we find a Ld_A, we find if the src1 already has a value in the local valueTable, if not we use the value from the pred's valueTable merge
-    // For other instrs, we assign null value
-    GlobHashTable * localSymToValueMap = GlobHashTable::New(globOpt->alloc, 8);
-    BVSparse<JitArenaAllocator> currentPathDefines(globOpt->alloc);
     IR::Instr *currentInlineeEnd = nullptr, *unskippedInlineeEnd = nullptr;
+    GlobHashTable * localSymToValueMap = nullptr;
+    BVSparse<JitArenaAllocator> * currentPathDefines = nullptr;
 
     auto UpdateValueForCopyTypeInstr = [&](IR::Instr *instr) -> Value* {
         Value * dstValue = nullptr;
@@ -4688,6 +4684,50 @@ BasicBlock::CheckLegalityAndFoldPathDepBranches(GlobOpt* globOpt)
 
     IR::Instr * instr = this->GetLastInstr();
 
+    // We have to first check the legality and only then allocate expensive data structures on the tempArena, because most block will have instructions we cant skip
+
+    while (instr)
+    {
+        if (!instr->IsBranchInstr() && !instr->IsLabelInstr() && !IsLegalOpcodeForPathDepBrFold(instr))
+        {
+            return;
+        }
+        if (instr->IsLabelInstr())
+        {
+            if (instr->AsLabelInstr()->m_isLoopTop)
+            {
+                // don't cross over to loops
+                return;
+            }
+        }
+        if (instr->IsBranchInstr())
+        {
+            IR::BranchInstr *branch = instr->AsBranchInstr();
+            if (branch->IsUnconditional())
+            {
+                if (!branch->GetTarget())
+                {
+                    return;
+                }
+                instr = branch->GetTarget();
+            }
+            else
+            {
+                // Found only legal instructions until a conditional branch, build expensive data structures and check provability
+                break;
+            }
+        }
+        else
+        {
+            instr = instr->m_next;
+        }
+    }
+
+    instr = this->GetLastInstr();
+    // Allocate hefty structures, we will not free them because OptBlock does a Reset on the tempAlloc
+    localSymToValueMap = GlobHashTable::New(globOpt->tempAlloc, 8);
+    currentPathDefines = JitAnew(globOpt->tempAlloc, BVSparse<JitArenaAllocator>, globOpt->tempAlloc);
+
     /* We start from the current instruction and go on scanning for legality, as long as it is legal to skip an instruction, skip.
      * When we see an unconditional branch, start scanning from the branchTarget
      * When we see a conditional branch, check if we can prove the branch target, if we can, adjust the flowgraph, and continue in the direction of the proven target
@@ -4703,6 +4743,14 @@ BasicBlock::CheckLegalityAndFoldPathDepBranches(GlobOpt* globOpt)
         {
             return;
         }
+        if (instr->IsLabelInstr())
+        {
+            if (instr->AsLabelInstr()->m_isLoopTop)
+            {
+                // don't cross over to loops
+                return;
+            }
+        }
         if (instr->m_opcode == Js::OpCode::InlineeEnd)
         {
             if (currentInlineeEnd != nullptr)
@@ -4719,7 +4767,7 @@ BasicBlock::CheckLegalityAndFoldPathDepBranches(GlobOpt* globOpt)
             }
 
             IR::RegOpnd *dst = instr->GetDst()->AsRegOpnd();
-            currentPathDefines.Set(dst->GetStackSym()->m_id);
+            currentPathDefines->Set(dst->GetStackSym()->m_id);
 
             if (IsCopyTypeInstr(instr))
             {
@@ -4736,14 +4784,7 @@ BasicBlock::CheckLegalityAndFoldPathDepBranches(GlobOpt* globOpt)
                 *localDstValue = nullptr;
             }
         }
-        if (instr->IsLabelInstr())
-        {
-            if (instr->AsLabelInstr()->m_isLoopTop)
-            {
-                // don't cross over to loops
-                return;
-            }
-        }
+
         if (instr->IsBranchInstr())
         {
             IR::BranchInstr* branch = instr->AsBranchInstr();
@@ -4774,7 +4815,7 @@ BasicBlock::CheckLegalityAndFoldPathDepBranches(GlobOpt* globOpt)
                 }
             }
 
-            FOREACH_BITSET_IN_SPARSEBV(id, &currentPathDefines)
+            FOREACH_BITSET_IN_SPARSEBV(id, currentPathDefines)
             {
                 if (branchTarget->GetBasicBlock()->upwardExposedUses->Test(id))
                 {