Fixing bug with hoisting a type check bailout

Author: rajatd

lib/Backend/BailOut.cpp

@@ -292,7 +292,7 @@ BailOutRecord::BailOutRecord(uint32 bailOutOffset, uint bailOutCacheIndex, IR::B
     argOutOffsetInfo(nullptr), bailOutOffset(bailOutOffset),
     bailOutCount(0), polymorphicCacheIndex(bailOutCacheIndex), bailOutKind(kind),
     branchValueRegSlot(Js::Constants::NoRegister),
-    ehBailoutData(nullptr), m_bailOutRecordId(0)
+    ehBailoutData(nullptr), m_bailOutRecordId(0), type(Normal)
 #if DBG
     , inlineDepth(0)
 #endif
@@ -1279,9 +1279,9 @@ BailOutRecord::BailOutFromLoopBodyHelper(Js::JavascriptCallStackLayout * layout,
     return bailOutOffset;
 }
 
-void BailOutRecord::UpdatePolymorphicFieldAccess(Js::JavascriptFunction *  function, BailOutRecord const * bailOutRecord)
+void BailOutRecord::UpdatePolymorphicFieldAccess(Js::JavascriptFunction * function, BailOutRecord const * bailOutRecord)
 {
-    Js::FunctionBody * executeFunction = function->GetFunctionBody();
+    Js::FunctionBody * executeFunction = bailOutRecord->type == Shared ? ((SharedBailOutRecord*)bailOutRecord)->functionBody : function->GetFunctionBody();
     Js::DynamicProfileInfo *dynamicProfileInfo = nullptr;
     if (executeFunction->HasDynamicProfileInfo())
     {
@@ -1290,7 +1290,7 @@ void BailOutRecord::UpdatePolymorphicFieldAccess(Js::JavascriptFunction *  funct
 
         if (bailOutRecord->polymorphicCacheIndex != (uint)-1)
         {
-            dynamicProfileInfo->RecordPolymorphicFieldAccess(function->GetFunctionBody(), bailOutRecord->polymorphicCacheIndex);
+            dynamicProfileInfo->RecordPolymorphicFieldAccess(executeFunction, bailOutRecord->polymorphicCacheIndex);
             if (IR::IsEquivalentTypeCheckBailOutKind(bailOutRecord->bailOutKind))
             {
                 // If we've already got a polymorphic inline cache, and if we've got an equivalent type check
@@ -2546,6 +2546,7 @@ BranchBailOutRecord::BranchBailOutRecord(uint32 trueBailOutOffset, uint32 falseB
     : BailOutRecord(trueBailOutOffset, (uint)-1, kind, bailOutFunc), falseBailOutOffset(falseBailOutOffset)
 {
     branchValueRegSlot = resultByteCodeReg;
+    type = BailoutRecordType::Branch;
 };
 
 Js::Var BranchBailOutRecord::BailOut(BranchBailOutRecord const * bailOutRecord, BOOL cond)
@@ -2633,6 +2634,13 @@ BranchBailOutRecord::BailOutFromLoopBodyInlined(Js::JavascriptCallStackLayout *
     return __super::BailOutFromLoopBodyInlinedCommon(layout, bailOutRecord, bailOutOffset, returnAddress, bailOutRecord->bailOutKind, branchValue);
 }
 
+SharedBailOutRecord::SharedBailOutRecord(uint32 bailOutOffset, uint bailOutCacheIndex, IR::BailOutKind kind, Func *bailOutFunc)
+    : BailOutRecord(bailOutOffset, bailOutCacheIndex, kind, bailOutFunc)
+{
+    this->functionBody = nullptr;
+    this->type = BailoutRecordType::Shared;
+}
+
 void LazyBailOutRecord::SetBailOutKind()
 {
     this->bailoutRecord->SetBailOutKind(IR::BailOutKind::LazyBailOut);

lib/Backend/BailOut.h

@@ -200,6 +200,13 @@ class BailOutRecord
     template <typename Fn>
     void MapArgOutOffsets(Fn fn);
 
+    enum BailoutRecordType : byte
+    {
+        Normal = 0,
+        Branch = 1,
+        Shared = 2
+    };
+    BailoutRecordType GetType() { return type; }
 protected:
     struct BailOutReturnValue
     {
@@ -308,6 +315,7 @@ class BailOutRecord
     void DumpLocalOffsets(uint count, int argOutSlotStart);
     void DumpValue(int offset, bool isFloat64);
 #endif
+    BailoutRecordType type;
     ushort bailOutCount;
     uint32 m_bailOutRecordId;
 
@@ -338,13 +346,13 @@ class BranchBailOutRecord : public BailOutRecord
     uint falseBailOutOffset;
 };
 
-class FunctionBailOutRecord
+class SharedBailOutRecord : public BailOutRecord
 {
 public:
-    FunctionBailOutRecord() : constantCount(0), constants(nullptr) {}
+    Js::FunctionBody* functionBody; // function body in which the bailout originally was before possible hoisting
 
-    uint constantCount;
-    Js::Var * constants;
+    SharedBailOutRecord(uint32 bailOutOffset, uint bailOutCacheIndex, IR::BailOutKind kind, Func *bailOutFunc);
+    static size_t GetOffsetOfFunctionBody() { return offsetof(SharedBailOutRecord, functionBody); }
 };
 
 template <typename Fn>

lib/Backend/Lower.cpp

@@ -12195,13 +12195,20 @@ Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::L
             // Generate code to write the cache index into the bailout record before we jump to the call site.
             Assert(bailOutInfo->polymorphicCacheIndex != (uint)-1);
             Assert(bailOutInfo->bailOutRecord);
-
             IR::MemRefOpnd *pIndexOpnd =
                 IR::MemRefOpnd::New((BYTE*)bailOutInfo->bailOutRecord + BailOutRecord::GetOffsetOfPolymorphicCacheIndex(), TyUint32, this->m_func);
             m_lowererMD.CreateAssign(
                 pIndexOpnd, IR::IntConstOpnd::New(bailOutInfo->polymorphicCacheIndex, TyUint32, this->m_func), instr);
         }
 
+        if (bailOutInfo->bailOutRecord->GetType() == BailOutRecord::BailoutRecordType::Shared)
+        {
+            IR::MemRefOpnd *functionBodyOpnd =
+                IR::MemRefOpnd::New((BYTE*)bailOutInfo->bailOutRecord + SharedBailOutRecord::GetOffsetOfFunctionBody(), TyMachPtr, this->m_func);
+            m_lowererMD.CreateAssign(
+                functionBodyOpnd, CreateFunctionBodyOpnd(instr->m_func), instr);
+        }
+
         // GenerateBailOut should have replaced this as a label as we should have already lowered
         // the main bailOutInstr.
         IR::LabelInstr * bailOutTargetLabel = bailOutInstr->AsLabelInstr();
@@ -12313,8 +12320,16 @@ Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::L
     }
     else
     {
-        bailOutRecord = NativeCodeDataNewZ(this->m_func->GetNativeCodeDataAllocator(),
-            BailOutRecord, bailOutInfo->bailOutOffset, bailOutInfo->polymorphicCacheIndex, instr->GetBailOutKind(), bailOutInfo->bailOutFunc);
+        if (bailOutInstr->GetBailOutKind() == IR::BailOutShared)
+        {
+            bailOutRecord = NativeCodeDataNewZ(this->m_func->GetNativeCodeDataAllocator(),
+                SharedBailOutRecord, bailOutInfo->bailOutOffset, bailOutInfo->polymorphicCacheIndex, instr->GetBailOutKind(), bailOutInfo->bailOutFunc);
+        }
+        else
+        {
+            bailOutRecord = NativeCodeDataNewZ(this->m_func->GetNativeCodeDataAllocator(),
+                BailOutRecord, bailOutInfo->bailOutOffset, bailOutInfo->polymorphicCacheIndex, instr->GetBailOutKind(), bailOutInfo->bailOutFunc);
+        }
 
         helperMethod = IR::HelperSaveAllRegistersAndBailOut;
 #ifdef _M_IX86

lib/Runtime/Base/FunctionBody.h

@@ -9,7 +9,6 @@
 
 struct CodeGenWorkItem;
 class SourceContextInfo;
-class FunctionBailOutRecord;
 struct DeferredFunctionStub;
 #ifdef DYNAMIC_PROFILE_MUTATOR
 class DynamicProfileMutator;

test/Optimizer/bug7100343.baseline renamed to test/Optimizer/forceRejitBugs.baseline

@@ -10,4 +10,31 @@ function foo(i1)
 }
 print(foo());
 print(foo());
-print(foo());
+print(foo());
+
+function test0() {
+}
+var arrObj0 = {};
+var func2 = function () {
+  Object;
+  function v0() {
+    for (var v1 = 0; v1 < 8; v1++) {
+      function func9() {
+        Object.prototype;
+      }
+      obj7 = func9();
+    }
+  }
+  v0();
+};
+
+var ary = Array(2);
+var VarArr0 = [];
+var __loopvar0 = 4;
+func2()
+func2()
+
+for (var _strvar65 of ary) {
+  prop5 = test0;
+  litObj9 = { prop7: VarArr0.unshift(func2(), func2()) };
+}

test/Optimizer/bug7100343.js renamed to test/Optimizer/forceRejitBugs.js

@@ -1361,9 +1361,9 @@
   </test>
   <test>
     <default>
-      <files>bug7100343.js</files>
+      <files>forceRejitBugs.js</files>
       <compile-flags>-mic:1 -off:simplejit -force:rejit</compile-flags>
-      <baseline>bug7100343.baseline</baseline>
+      <baseline>forceRejitBugs.baseline</baseline>
     </default>
   </test>
 </regress-exe>