[1.4>2.0] [MERGE #2538 @ThomsonTan] Fix operand types after hoisting IR instrs to landingpad

Merge pull request #2538 from ThomsonTan:FixHoistCheckFixedFld

We don't fix value type of many IR instructions when hositing it into landingpad (like for `CheckFixFld`, we did it for `FromVar`). Then incorrect value type info in landingpad causes bad codegen, like missing some type checks required by the type info in landingpad (some type assertions in the loop are false in the landingpad because they are from bailout in the loop) , and the missing checked causes access violation finally.

I got this problem from crawler. The fix and simple repro was mostly made by Louis. Thanks @LouisLaf.

lib/Backend/GlobOpt.cpp

@@ -19050,6 +19050,44 @@ GlobOpt::OptDstIsInvariant(IR::RegOpnd *dst)
     return (dstSym->m_isSingleDef);
 }
 
+void
+GlobOpt::OptHoistToLandingPadUpdateValueType(
+    BasicBlock* landingPad,
+    IR::Instr* instr,
+    IR::Opnd* opnd,
+    Value* opndVal)
+{
+    if (instr->m_opcode == Js::OpCode::FromVar)
+    {
+        return;
+    }
+
+    Sym* opndSym = opnd->GetSym();;
+
+    if (opndSym)
+    {
+        if (opndVal == nullptr)
+        {
+            opndVal = FindValue(opndSym);
+        }
+
+        Value* opndValueInLandingPad = FindValue(landingPad->globOptData.symToValueMap, opndSym);
+        Assert(opndVal->GetValueNumber() == opndValueInLandingPad->GetValueNumber());
+
+        opnd->SetValueType(opndValueInLandingPad->GetValueInfo()->Type());
+
+        if (opndSym->IsPropertySym())
+        {
+            // Also fix valueInfo on objPtr
+            StackSym* opndObjPtrSym = opndSym->AsPropertySym()->m_stackSym;
+            Value* opndObjPtrSymValInLandingPad = FindValue(landingPad->globOptData.symToValueMap, opndObjPtrSym);
+            ValueInfo* opndObjPtrSymValueInfoInLandingPad = opndObjPtrSymValInLandingPad->GetValueInfo();
+
+            opnd->AsSymOpnd()->SetPropertyOwnerValueType(opndObjPtrSymValueInfoInLandingPad->Type());
+        }
+    }
+}
+
 void
 GlobOpt::OptHoistInvariant(
     IR::Instr *instr,
@@ -19062,6 +19100,30 @@ GlobOpt::OptHoistInvariant(
     IR::BailOutKind bailoutKind)
 {
     BasicBlock *landingPad = loop->landingPad;
+
+    IR::Opnd* src1 = instr->GetSrc1();
+    if (src1)
+    {
+        // We are hoisting this instruction possibly past other uses, which might invalidate the last use info. Clear it.
+        OptHoistToLandingPadUpdateValueType(landingPad, instr, src1, src1Val);
+
+        if (src1->IsRegOpnd())
+        {
+            src1->AsRegOpnd()->m_isTempLastUse = false;
+        }
+
+        IR::Opnd* src2 = instr->GetSrc2();
+        if (src2)
+        {
+            OptHoistToLandingPadUpdateValueType(landingPad, instr, src2, nullptr);
+
+            if (src2->IsRegOpnd())
+            {
+                src2->AsRegOpnd()->m_isTempLastUse = false;
+            }
+        }
+    }
+
     IR::RegOpnd *dst = instr->GetDst() ? instr->GetDst()->AsRegOpnd() : nullptr;
     if(dst)
     {
@@ -19303,26 +19365,6 @@ GlobOpt::OptHoistInvariant(
         }
     }
 
-    if (instr->GetSrc1())
-    {
-        // We are hoisting this instruction possibly past other uses, which might invalidate the last use info. Clear it.
-        IR::Opnd *src1 = instr->GetSrc1();
-
-        if (src1->IsRegOpnd())
-        {
-            src1->AsRegOpnd()->m_isTempLastUse = false;
-        }
-        if (instr->GetSrc2())
-        {
-            IR::Opnd *src2 = instr->GetSrc2();
-
-            if (src2->IsRegOpnd())
-            {
-                src2->AsRegOpnd()->m_isTempLastUse = false;
-            }
-        }
-    }
-
     if(!dst)
     {
         return;

lib/Backend/GlobOpt.h

@@ -1605,6 +1605,7 @@ private:
     bool                    TryHoistInvariant(IR::Instr *instr, BasicBlock *block, Value *dstVal, Value *src1Val, Value *src2Val, bool isNotTypeSpecConv,
                                                 const bool lossy = false, const bool forceInvariantHoisting = false, IR::BailOutKind bailoutKind = IR::BailOutInvalid);
     void                    HoistInvariantValueInfo(ValueInfo *const invariantValueInfoToHoist, Value *const valueToUpdate, BasicBlock *const targetBlock);
+    void                    OptHoistToLandingPadUpdateValueType(BasicBlock* landingPad, IR::Instr* instr, IR::Opnd* opnd, Value *const srcVal);
 public:
     static bool             IsTypeSpecPhaseOff(Func* func);
     static bool             DoAggressiveIntTypeSpec(Func* func);

lib/Backend/Opnd.cpp

@@ -396,6 +396,20 @@ Opnd::GetStackSym() const
     }
 }
 
+Sym*
+Opnd::GetSym() const
+{
+    switch (this->GetKind())
+    {
+        case OpndKindSym:
+            return static_cast<SymOpnd const *>(this)->m_sym;
+        case OpndKindReg:
+            return static_cast<RegOpnd const *>(this)->m_sym;
+        default:
+            return nullptr;
+    }
+}
+
 int64
 Opnd::GetImmediateValue(Func* func)
 {

lib/Backend/Opnd.h

@@ -191,6 +191,7 @@ public:
     Opnd *              CloneDef(Func *func);
     Opnd *              CloneUse(Func *func);
     StackSym *          GetStackSym() const;
+    Sym *               GetSym() const;
     Opnd *              UseWithNewType(IRType type, Func * func);
 
     bool                IsEqual(Opnd *opnd);

test/Optimizer/FixTypeAfterHoisting.js

@@ -0,0 +1,21 @@
+//-------------------------------------------------------------------------------------------------------
+// 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(a)
+{
+    for (var i = a.length; i--; )
+    {
+        c = a[i];
+        b =a.slice(i);
+    }
+}
+
+var arr = [0,1,2,3];
+
+test0(arr);
+test0(0);
+
+print('pass');
+

test/Optimizer/rlexe.xml

@@ -1372,4 +1372,10 @@
       <baseline>negativeZero_bugs.baseline</baseline>
     </default>
   </test>
+  <test>
+    <default>
+      <files>FixTypeAfterHoisting.js</files>
+      <compile-flags>-lic:1 -off:simplejit -off:aggressiveinttypespec -bgjit-</compile-flags>
+    </default>
+  </test>
 </regress-exe>