[Merge chakra-core/ChakraCore@f497760a25] [1.6>1.7] [MERGE #3460 @meg…

…-gupta] Fix order of generating bailout info in ValueNumberLdElem

Merge pull request #3460 from meg-gupta:fixldelemvaluenumber

When there is an early return in try finally, we have to execute the finally block before returning
So we add edges from early return block to finally block and finally block back to early return block.

If an instruction assigning to s0 is type specialized and s0 already exists in bytecodeUpwardExposedUsed
While populating bailout info, we will end up using the type specialized symbol instead of s0 even for pre opt bailout.
Because we don't differentiate between pre-op bailout and post-op bailout while populating bailout info
The type specialized symbol gets added on bytecodeUpwardExposed used, due to this gets added to upwardExposedUsed
If this instruction was in a loop, gets added to liveOnBackEdge syms.
This causes asserts in register allocator, when it extends the lifetime of the type specialized symbol to the entire loop.

This is a problem only with try-finallys. For non-EH functions, the early return block gets moved out of the loop due to break block removal.
With try finallys, these blocks are not moved out due to the early return -> finally and finally -> early return edges.

This happens only with return s0 symbol, because we avoid creating new temporary destination register for the s0 case.

This happens only in ValueNumberLdElem because it first calls TypeSpecializeIntDst and TypeSpecializeFloatDst and then calls GenerateBailAtOperation for the pre-op bailout.

deps/chakrashim/core/lib/Backend/GlobOpt.cpp

@@ -5350,6 +5350,39 @@ GlobOpt::ValueNumberLdElemDst(IR::Instr **pInstr, Value *srcVal)
             }
             return dstVal;
         }
+
+        if (!this->IsLoopPrePass())
+        {
+            if (instr->HasBailOutInfo())
+            {
+                const IR::BailOutKind oldBailOutKind = instr->GetBailOutKind();
+                Assert(
+                    (
+                        !(oldBailOutKind & ~IR::BailOutKindBits) ||
+                        (oldBailOutKind & ~IR::BailOutKindBits) == IR::BailOutOnImplicitCallsPreOp
+                        ) &&
+                    !(oldBailOutKind & IR::BailOutKindBits & ~(IR::BailOutOnArrayAccessHelperCall | IR::BailOutMarkTempObject)));
+                if (bailOutKind == IR::BailOutConventionalTypedArrayAccessOnly)
+                {
+                    // BailOutConventionalTypedArrayAccessOnly also bails out if the array access is outside the head
+                    // segment bounds, and guarantees no implicit calls. Override the bailout kind so that the instruction
+                    // bails out for the right reason.
+                    instr->SetBailOutKind(
+                        bailOutKind | (oldBailOutKind & (IR::BailOutKindBits - IR::BailOutOnArrayAccessHelperCall)));
+                }
+                else
+                {
+                    // BailOutConventionalNativeArrayAccessOnly by itself may generate a helper call, and may cause implicit
+                    // calls to occur, so it must be merged in to eliminate generating the helper call
+                    Assert(bailOutKind == IR::BailOutConventionalNativeArrayAccessOnly);
+                    instr->SetBailOutKind(oldBailOutKind | bailOutKind);
+                }
+            }
+            else
+            {
+                GenerateBailAtOperation(&instr, bailOutKind);
+            }
+        }
         TypeSpecializeIntDst(instr, instr->m_opcode, nullptr, nullptr, nullptr, bailOutKind, newMin, newMax, &dstVal);
         toType = TyInt32;
         break;
@@ -5379,6 +5412,39 @@ GlobOpt::ValueNumberLdElemDst(IR::Instr **pInstr, Value *srcVal)
             }
             return dstVal;
         }
+
+        if (!this->IsLoopPrePass())
+        {
+            if (instr->HasBailOutInfo())
+            {
+                const IR::BailOutKind oldBailOutKind = instr->GetBailOutKind();
+                Assert(
+                    (
+                        !(oldBailOutKind & ~IR::BailOutKindBits) ||
+                        (oldBailOutKind & ~IR::BailOutKindBits) == IR::BailOutOnImplicitCallsPreOp
+                        ) &&
+                    !(oldBailOutKind & IR::BailOutKindBits & ~(IR::BailOutOnArrayAccessHelperCall | IR::BailOutMarkTempObject)));
+                if (bailOutKind == IR::BailOutConventionalTypedArrayAccessOnly)
+                {
+                    // BailOutConventionalTypedArrayAccessOnly also bails out if the array access is outside the head
+                    // segment bounds, and guarantees no implicit calls. Override the bailout kind so that the instruction
+                    // bails out for the right reason.
+                    instr->SetBailOutKind(
+                        bailOutKind | (oldBailOutKind & (IR::BailOutKindBits - IR::BailOutOnArrayAccessHelperCall)));
+                }
+                else
+                {
+                    // BailOutConventionalNativeArrayAccessOnly by itself may generate a helper call, and may cause implicit
+                    // calls to occur, so it must be merged in to eliminate generating the helper call
+                    Assert(bailOutKind == IR::BailOutConventionalNativeArrayAccessOnly);
+                    instr->SetBailOutKind(oldBailOutKind | bailOutKind);
+                }
+            }
+            else
+            {
+                GenerateBailAtOperation(&instr, bailOutKind);
+            }
+        }
         TypeSpecializeFloatDst(instr, nullptr, nullptr, nullptr, &dstVal);
         toType = TyFloat64;
         break;
@@ -5426,39 +5492,6 @@ GlobOpt::ValueNumberLdElemDst(IR::Instr **pInstr, Value *srcVal)
         Output::Flush();
     }
 
-    if(!this->IsLoopPrePass())
-    {
-        if(instr->HasBailOutInfo())
-        {
-            const IR::BailOutKind oldBailOutKind = instr->GetBailOutKind();
-            Assert(
-                (
-                    !(oldBailOutKind & ~IR::BailOutKindBits) ||
-                    (oldBailOutKind & ~IR::BailOutKindBits) == IR::BailOutOnImplicitCallsPreOp
-                ) &&
-                !(oldBailOutKind & IR::BailOutKindBits & ~(IR::BailOutOnArrayAccessHelperCall | IR::BailOutMarkTempObject)));
-            if(bailOutKind == IR::BailOutConventionalTypedArrayAccessOnly)
-            {
-                // BailOutConventionalTypedArrayAccessOnly also bails out if the array access is outside the head
-                // segment bounds, and guarantees no implicit calls. Override the bailout kind so that the instruction
-                // bails out for the right reason.
-                instr->SetBailOutKind(
-                    bailOutKind | (oldBailOutKind & (IR::BailOutKindBits - IR::BailOutOnArrayAccessHelperCall)));
-            }
-            else
-            {
-                // BailOutConventionalNativeArrayAccessOnly by itself may generate a helper call, and may cause implicit
-                // calls to occur, so it must be merged in to eliminate generating the helper call
-                Assert(bailOutKind == IR::BailOutConventionalNativeArrayAccessOnly);
-                instr->SetBailOutKind(oldBailOutKind | bailOutKind);
-            }
-        }
-        else
-        {
-            GenerateBailAtOperation(&instr, bailOutKind);
-        }
-    }
-
     return dstVal;
 }
 

deps/chakrashim/core/test/EH/rlexe.xml

@@ -118,4 +118,10 @@
       <files>tryfinallytests.js</files>
     </default>
   </test>
+  <test>
+    <default>
+      <files>tryfinallyldelembug.js</files>
+      <compile-flags> -off:arraycheckhoist </compile-flags>
+    </default>
+  </test>
 </regress-exe>

deps/chakrashim/core/test/EH/tryfinallyldelembug.js

@@ -0,0 +1,17 @@
+//-------------------------------------------------------------------------------------------------------
+// Copyright (C) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
+//-------------------------------------------------------------------------------------------------------
+
+function test0() {
+  var i32 = new Int32Array();
+  while (parseInt()) {
+    for (var _strvar0 of uic8) {
+      return i32[66];
+    }
+  }
+}
+test0();
+test0();
+test0();
+WScript.Echo("passed");