Explorar el Código

[MERGE #4995 @rajatd] Aggressive Value Transfer in loop prepass

Merge pull request #4995 from rajatd:preInline

This change enables us to transfer values in the prepass of a loop. This enables us to do more aggressive object pointer copy-prop in the prepass, which is a precursor to getting field PRE kick in in more cases.

The basic condition that needs to be satisfied to transfer value from `src` to `dst` in the pre-pass is that either the `src` shouldn't be live on back edge of the current loop or, if it is, it shouldn't be written to in the loop (or any of the parent loops).

I have also included a change that will enable field PRE in cases when the `this` object is passed to an inlinee that loads properties from that object. Those loads could now be PRE'd.
Rajat Dua hace 7 años
padre
commit
10acdfa7e0

+ 11 - 5
lib/Backend/BailOut.cpp

@@ -1217,7 +1217,7 @@ BailOutRecord::BailOutFromLoopBodyHelper(Js::JavascriptCallStackLayout * layout,
 
 void BailOutRecord::UpdatePolymorphicFieldAccess(Js::JavascriptFunction * function, BailOutRecord const * bailOutRecord)
 {
-    Js::FunctionBody * executeFunction = bailOutRecord->type == Shared ? ((SharedBailOutRecord*)bailOutRecord)->functionBody : function->GetFunctionBody();
+    Js::FunctionBody * executeFunction = bailOutRecord->IsShared() ? ((SharedBailOutRecord*)bailOutRecord)->functionBody : function->GetFunctionBody();
     Js::DynamicProfileInfo *dynamicProfileInfo = nullptr;
     if (executeFunction->HasDynamicProfileInfo())
     {
@@ -1714,11 +1714,10 @@ void BailOutRecord::ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::S
 
     Assert(bailOutKind != IR::BailOutInvalid);
 
-    if ((executeFunction->HasDynamicProfileInfo() && callsCount == 0) ||
+    Js::DynamicProfileInfo * profileInfo = executeFunction->HasDynamicProfileInfo() ? executeFunction->GetAnyDynamicProfileInfo() : nullptr;
+    if ((profileInfo && callsCount == 0) ||
         PHASE_FORCE(Js::ReJITPhase, executeFunction))
     {
-        Js::DynamicProfileInfo * profileInfo = executeFunction->GetAnyDynamicProfileInfo();
-
         if ((bailOutKind & (IR::BailOutOnResultConditions | IR::BailOutOnDivSrcConditions)) || bailOutKind == IR::BailOutIntOnly || bailOutKind == IR::BailOnIntMin || bailOutKind == IR::BailOnDivResultNotInt)
         {
             // Note WRT BailOnIntMin: it wouldn't make sense to re-jit without changing anything here, as interpreter will not change the (int) type,
@@ -2161,7 +2160,6 @@ void BailOutRecord::ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::S
 
     if (!reThunk && rejitReason != RejitReason::None)
     {
-        Js::DynamicProfileInfo * profileInfo = executeFunction->GetAnyDynamicProfileInfo();
         // REVIEW: Temporary fix for RS1.  Disable Rejiting if it looks like it is not fixing the problem.
         //         For RS2, turn the rejitCount check into an assert and let's fix all these issues.
         if (profileInfo->GetRejitCount() >= 100 ||
@@ -2229,6 +2227,14 @@ void BailOutRecord::ScheduleFunctionCodeGen(Js::ScriptFunction * function, Js::S
     }
     else if (rejitReason != RejitReason::None)
     {
+        if (bailOutRecord->IsForLoopTop() && IR::IsTypeCheckBailOutKind(bailOutRecord->bailOutKind))
+        {
+            // Disable FieldPRE if we're triggering a type check rejit due to a bailout at the loop top.
+            // Most likely this was caused by a CheckFixedFld that was hoisted from a branch block where 
+            // only certain types flowed, to the loop top, where more types (different or non-equivalent)
+            // were flowing in.
+            profileInfo->DisableFieldPRE();
+        }
 #ifdef REJIT_STATS
         executeFunction->GetScriptContext()->LogRejit(executeFunction, rejitReason);
 #endif

+ 8 - 3
lib/Backend/BailOut.h

@@ -27,8 +27,8 @@ public:
     BailOutInfo(uint32 bailOutOffset, Func* bailOutFunc) :
         bailOutOffset(bailOutOffset), bailOutFunc(bailOutFunc),
         byteCodeUpwardExposedUsed(nullptr), polymorphicCacheIndex((uint)-1), startCallCount(0), startCallInfo(nullptr), bailOutInstr(nullptr),
-        totalOutParamCount(0), argOutSyms(nullptr), bailOutRecord(nullptr), wasCloned(false), isInvertedBranch(false), sharedBailOutKind(true), outParamInlinedArgSlot(nullptr),
-        liveVarSyms(nullptr), liveLosslessInt32Syms(nullptr), liveFloat64Syms(nullptr),
+        totalOutParamCount(0), argOutSyms(nullptr), bailOutRecord(nullptr), wasCloned(false), isInvertedBranch(false), sharedBailOutKind(true), isLoopTopBailOutInfo(false),
+        outParamInlinedArgSlot(nullptr), liveVarSyms(nullptr), liveLosslessInt32Syms(nullptr), liveFloat64Syms(nullptr),
         branchConditionOpnd(nullptr),
         stackLiteralBailOutInfoCount(0), stackLiteralBailOutInfo(nullptr)
     {
@@ -70,6 +70,7 @@ public:
     bool wasCloned;
     bool isInvertedBranch;
     bool sharedBailOutKind;
+    bool isLoopTopBailOutInfo;
 
 #if DBG
     bool wasCopied;
@@ -201,9 +202,13 @@ public:
     {
         Normal = 0,
         Branch = 1,
-        Shared = 2
+        Shared = 2,
+        SharedForLoopTop = 3
     };
     BailoutRecordType GetType() { return type; }
+    void SetType(BailoutRecordType type) { this->type = type; }
+    bool IsShared() const { return type == Shared || type == SharedForLoopTop; }
+    bool IsForLoopTop() const { return type == SharedForLoopTop; }
 protected:
     struct BailOutReturnValue
     {

+ 40 - 34
lib/Backend/FlowGraph.cpp

@@ -3345,6 +3345,19 @@ BasicBlock::IsLandingPad()
     return nextBlock && nextBlock->loop && nextBlock->isLoopHeader && nextBlock->loop->landingPad == this;
 }
 
+BailOutInfo *
+BasicBlock::CreateLoopTopBailOutInfo(GlobOpt * globOpt)
+{
+    IR::Instr * firstInstr = this->GetFirstInstr();
+    BailOutInfo* bailOutInfo = JitAnew(globOpt->func->m_alloc, BailOutInfo, firstInstr->GetByteCodeOffset(), firstInstr->m_func);
+    bailOutInfo->isLoopTopBailOutInfo = true;
+    globOpt->FillBailOutInfo(this, bailOutInfo);
+#if ENABLE_DEBUG_CONFIG_OPTIONS
+    bailOutInfo->bailOutOpcode = Js::OpCode::LoopBodyStart;
+#endif
+    return bailOutInfo;
+}
+
 IR::Instr *
 FlowGraph::RemoveInstr(IR::Instr *instr, GlobOpt * globOpt)
 {
@@ -4227,8 +4240,9 @@ BasicBlock::CleanUpValueMaps()
     {
         FOREACH_SLISTBASE_ENTRY_EDITING(GlobHashBucket, bucket, &thisTable->table[i], iter)
         {
-            bool isSymUpwardExposed = upwardExposedUses->Test(bucket.value->m_id) || upwardExposedFields->Test(bucket.value->m_id);
-            if (!isSymUpwardExposed && symsInCallSequence.Test(bucket.value->m_id))
+            Sym * sym = bucket.value;
+            bool isSymUpwardExposed = upwardExposedUses->Test(sym->m_id) || upwardExposedFields->Test(sym->m_id);
+            if (!isSymUpwardExposed && symsInCallSequence.Test(sym->m_id))
             {
                 // Don't remove/shrink sym-value pair if the sym is referenced in callSequence even if the sym is dead according to backward data flow.
                 // This is possible in some edge cases that an infinite loop is involved when evaluating parameter for a function (between StartCall and Call),
@@ -4241,22 +4255,22 @@ BasicBlock::CleanUpValueMaps()
             // Make sure symbol was created before backward pass.
             // If symbols isn't upward exposed, mark it as dead.
             // If a symbol was copy-prop'd in a loop prepass, the upwardExposedUses info could be wrong.  So wait until we are out of the loop before clearing it.
-            if ((SymID)bucket.value->m_id <= this->globOptData.globOpt->maxInitialSymID && !isSymUpwardExposed
-                && (!isInLoop || !this->globOptData.globOpt->prePassCopyPropSym->Test(bucket.value->m_id)))
+            bool isSymFieldPRESymStore = isInLoop && this->loop->fieldPRESymStores->Test(sym->m_id);
+            if ((SymID)sym->m_id <= this->globOptData.globOpt->maxInitialSymID && !isSymUpwardExposed && !isSymFieldPRESymStore
+                && (!isInLoop || !this->globOptData.globOpt->prePassCopyPropSym->Test(sym->m_id)))
             {
                 Value *val = bucket.element;
                 ValueInfo *valueInfo = val->GetValueInfo();
 
-                Sym * sym = bucket.value;
                 Sym *symStore = valueInfo->GetSymStore();
 
-                if (symStore && symStore == bucket.value)
+                if (symStore && symStore == sym)
                 {
                     // Keep constants around, as we don't know if there will be further uses
                     if (!bucket.element->GetValueInfo()->IsVarConstant() && !bucket.element->GetValueInfo()->HasIntConstantValue())
                     {
                         // Symbol may still be a copy-prop candidate.  Wait before deleting it.
-                        deadSymsBv.Set(bucket.value->m_id);
+                        deadSymsBv.Set(sym->m_id);
 
                         // Make sure the type sym is added to the dead syms vector as well, because type syms are
                         // created in backward pass and so their symIds > maxInitialSymID.
@@ -4287,8 +4301,6 @@ BasicBlock::CleanUpValueMaps()
             }
             else
             {
-                Sym * sym = bucket.value;
-
                 if (sym->IsPropertySym() && !this->globOptData.liveFields->Test(sym->m_id))
                 {
                     // Remove propertySyms which are not live anymore.
@@ -4306,7 +4318,7 @@ BasicBlock::CleanUpValueMaps()
 
                     Sym *symStore = valueInfo->GetSymStore();
 
-                    if (symStore && symStore != bucket.value)
+                    if (symStore && symStore != sym)
                     {
                         keepAliveSymsBv.Set(symStore->m_id);
                         if (symStore->IsStackSym() && symStore->AsStackSym()->HasObjectTypeSym())
@@ -4843,13 +4855,7 @@ BasicBlock::MergePredBlocksValueMaps(GlobOpt* globOpt)
         {
             // Capture bail out info in case we have optimization that needs it
             Assert(this->loop->bailOutInfo == nullptr);
-            IR::Instr * firstInstr = this->GetFirstInstr();
-            this->loop->bailOutInfo = JitAnew(globOpt->func->m_alloc, BailOutInfo,
-                firstInstr->GetByteCodeOffset(), firstInstr->m_func);
-            globOpt->FillBailOutInfo(this, this->loop->bailOutInfo);
-#if ENABLE_DEBUG_CONFIG_OPTIONS
-            this->loop->bailOutInfo->bailOutOpcode = Js::OpCode::LoopBodyStart;
-#endif
+            this->loop->bailOutInfo = this->CreateLoopTopBailOutInfo(globOpt);
         }
 
         // If loop pre-pass, don't insert convert from type-spec to var
@@ -4969,9 +4975,9 @@ BasicBlock::MergePredBlocksValueMaps(GlobOpt* globOpt)
     // (airlock block) to put in the conversion code.
     Assert(globOpt->tempBv->IsEmpty());
 
-    BVSparse<JitArenaAllocator> tempBv2(globOpt->tempAlloc);
-    BVSparse<JitArenaAllocator> tempBv3(globOpt->tempAlloc);
-    BVSparse<JitArenaAllocator> tempBv4(globOpt->tempAlloc);
+    BVSparse<JitArenaAllocator> symsNeedingLossyIntConversion(globOpt->tempAlloc);
+    BVSparse<JitArenaAllocator> symsNeedingLosslessIntConversion(globOpt->tempAlloc);
+    BVSparse<JitArenaAllocator> symsNeedingFloatConversion(globOpt->tempAlloc);
 
     FOREACH_PREDECESSOR_EDGE_EDITING(edge, this, iter)
     {
@@ -4993,22 +4999,22 @@ BasicBlock::MergePredBlocksValueMaps(GlobOpt* globOpt)
         }
 
         // Lossy int in the merged block, and no int in the predecessor - need a lossy conversion to int
-        tempBv2.Minus(blockData.liveLossyInt32Syms, pred->globOptData.liveInt32Syms);
+        symsNeedingLossyIntConversion.Minus(blockData.liveLossyInt32Syms, pred->globOptData.liveInt32Syms);
 
         // Lossless int in the merged block, and no lossless int in the predecessor - need a lossless conversion to int
-        tempBv3.Minus(blockData.liveInt32Syms, this->globOptData.liveLossyInt32Syms);
+        symsNeedingLosslessIntConversion.Minus(blockData.liveInt32Syms, blockData.liveLossyInt32Syms);
         globOpt->tempBv->Minus(pred->globOptData.liveInt32Syms, pred->globOptData.liveLossyInt32Syms);
-        tempBv3.Minus(globOpt->tempBv);
+        symsNeedingLosslessIntConversion.Minus(globOpt->tempBv);
 
         globOpt->tempBv->Minus(blockData.liveVarSyms, pred->globOptData.liveVarSyms);
-        tempBv4.Minus(blockData.liveFloat64Syms, pred->globOptData.liveFloat64Syms);
+        symsNeedingFloatConversion.Minus(blockData.liveFloat64Syms, pred->globOptData.liveFloat64Syms);
 
-        bool symIVNeedsSpecializing = (symIV && !pred->globOptData.liveInt32Syms->Test(symIV->m_id) && !tempBv3.Test(symIV->m_id));
+        bool symIVNeedsSpecializing = (symIV && !pred->globOptData.liveInt32Syms->Test(symIV->m_id) && !symsNeedingLosslessIntConversion.Test(symIV->m_id));
 
         if (!globOpt->tempBv->IsEmpty() ||
-            !tempBv2.IsEmpty() ||
-            !tempBv3.IsEmpty() ||
-            !tempBv4.IsEmpty() ||
+            !symsNeedingLossyIntConversion.IsEmpty() ||
+            !symsNeedingLosslessIntConversion.IsEmpty() ||
+            !symsNeedingFloatConversion.IsEmpty() ||
             symIVNeedsSpecializing ||
             symsRequiringCompensationToMergedValueInfoMap.Count() != 0)
         {
@@ -5051,17 +5057,17 @@ BasicBlock::MergePredBlocksValueMaps(GlobOpt* globOpt)
             {
                 globOpt->ToVar(globOpt->tempBv, pred);
             }
-            if (!tempBv2.IsEmpty())
+            if (!symsNeedingLossyIntConversion.IsEmpty())
             {
-                globOpt->ToInt32(&tempBv2, pred, true /* lossy */);
+                globOpt->ToInt32(&symsNeedingLossyIntConversion, pred, true /* lossy */);
             }
-            if (!tempBv3.IsEmpty())
+            if (!symsNeedingLosslessIntConversion.IsEmpty())
             {
-                globOpt->ToInt32(&tempBv3, pred, false /* lossy */);
+                globOpt->ToInt32(&symsNeedingLosslessIntConversion, pred, false /* lossy */);
             }
-            if (!tempBv4.IsEmpty())
+            if (!symsNeedingFloatConversion.IsEmpty())
             {
-                globOpt->ToFloat64(&tempBv4, pred);
+                globOpt->ToFloat64(&symsNeedingFloatConversion, pred);
             }
             if (symIVNeedsSpecializing)
             {

+ 2 - 1
lib/Backend/FlowGraph.h

@@ -346,6 +346,7 @@ public:
     }
 
     bool IsLandingPad();
+    BailOutInfo * CreateLoopTopBailOutInfo(GlobOpt * globOpt);
 
     // GlobOpt Stuff
 public:
@@ -596,7 +597,7 @@ public:
     ValueNumber         firstValueNumberInLoop;
     JsArrayKills        jsArrayKills;
     BVSparse<JitArenaAllocator> *fieldKilled;
-    BVSparse<JitArenaAllocator> *fieldPRESymStore;
+    BVSparse<JitArenaAllocator> *fieldPRESymStores;
     InitialValueFieldMap initialValueFieldMap;
 
     InductionVariableSet *inductionVariables;

+ 68 - 39
lib/Backend/GlobOpt.cpp

@@ -450,6 +450,11 @@ GlobOpt::OptBlock(BasicBlock *block)
         if (loop != this->prePassLoop)
         {
             OptLoops(loop);
+            if (!IsLoopPrePass() && loop->parent)
+            {
+                loop->fieldPRESymStores->Or(loop->parent->fieldPRESymStores);
+            }
+            
             if (!this->IsLoopPrePass() && DoFieldPRE(loop))
             {
                 // Note: !IsLoopPrePass means this was a root loop pre-pass. FieldPre() is called once per loop.
@@ -544,6 +549,7 @@ GlobOpt::OptBlock(BasicBlock *block)
                 if (succ->isLoopHeader && succ->loop->IsDescendentOrSelf(block->loop))
                 {
                     BVSparse<JitArenaAllocator> *liveOnBackEdge = block->loop->regAlloc.liveOnBackEdgeSyms;
+                    liveOnBackEdge->Or(block->loop->fieldPRESymStores);
 
                     this->tempBv->Minus(block->loop->varSymsOnEntry, block->globOptData.liveVarSyms);
                     this->tempBv->And(liveOnBackEdge);
@@ -664,7 +670,7 @@ GlobOpt::OptLoops(Loop *loop)
 
         loop->symsDefInLoop = JitAnew(this->alloc, BVSparse<JitArenaAllocator>, this->alloc);
         loop->fieldKilled = JitAnew(alloc, BVSparse<JitArenaAllocator>, this->alloc);
-        loop->fieldPRESymStore = JitAnew(alloc, BVSparse<JitArenaAllocator>, this->alloc);
+        loop->fieldPRESymStores = JitAnew(alloc, BVSparse<JitArenaAllocator>, this->alloc);
         loop->allFieldsKilled = false;
     }
     else
@@ -1036,13 +1042,13 @@ BOOL GlobOpt::PreloadPRECandidate(Loop *loop, GlobHashBucket* candidate)
     if (ldInstr->GetDst()->AsRegOpnd()->m_sym != symStore)
     {
         ldInstr->ReplaceDst(IR::RegOpnd::New(symStore->AsStackSym(), TyVar, this->func));
+        loop->fieldPRESymStores->Set(symStore->m_id);
     }
 
     ldInstr->GetSrc1()->SetIsJITOptimizedReg(true);
     ldInstr->GetDst()->SetIsJITOptimizedReg(true);
 
     landingPad->globOptData.liveVarSyms->Set(symStore->m_id);
-    loop->fieldPRESymStore->Set(symStore->m_id);
 
     ValueType valueType(ValueType::Uninitialized);
     Value *initialValue = nullptr;
@@ -2638,6 +2644,16 @@ GlobOpt::OptInstr(IR::Instr *&instr, bool* isInstrRemoved)
         this->InsertByteCodeUses(instr);
     }
 
+    if (!IsLoopPrePass() && instr->HasBailOutInfo())
+    {
+        // Aggregate byteCodeUpwardExposedUsed of preceding ByteCodeUses instrs with the same bytecode offset.
+        // This is required as different ByteCodeUses instrs may be inserted for an instr in the loop pre-pass
+        // and the main pass (and there may be additional instructions inserted between the two sets of 
+        // ByteCodeUses instructions), but the Backward Pass only processes immediately preceding consecutive
+        // ByteCodeUses instructions before processing a pre-op bailout.
+        instr->AggregateByteCodeUses();
+    }
+
     if (!this->IsLoopPrePass() && !isHoisted && this->IsImplicitCallBailOutCurrentlyNeeded(instr, src1Val, src2Val))
     {
         IR::BailOutKind kind = IR::BailOutOnImplicitCalls;
@@ -3028,11 +3044,10 @@ GlobOpt::SetLoopFieldInitialValue(Loop *loop, IR::Instr *instr, PropertySym *pro
     Value *initialValue = nullptr;
     StackSym *symStore;
 
-    if (loop->allFieldsKilled || loop->fieldKilled->Test(originalPropertySym->m_id))
+    if (loop->allFieldsKilled || loop->fieldKilled->Test(originalPropertySym->m_id) || loop->fieldKilled->Test(propertySym->m_id))
     {
         return;
     }
-    Assert(!loop->fieldKilled->Test(propertySym->m_id));
 
     // Value already exists
     if (CurrentBlockData()->FindValue(propertySym))
@@ -5354,7 +5369,7 @@ GlobOpt::GetPrepassValueTypeForDst(
     IR::Instr *const instr,
     Value *const src1Value,
     Value *const src2Value,
-    bool *const isValueInfoPreciseRef) const
+    bool const isValueInfoPrecise) const
 {
     // Values with definite types can be created in the loop prepass only when it is guaranteed that the value type will be the
     // same on any iteration of the loop. The heuristics currently used are:
@@ -5371,18 +5386,12 @@ GlobOpt::GetPrepassValueTypeForDst(
     Assert(IsLoopPrePass());
     Assert(instr);
 
-    if(isValueInfoPreciseRef)
-    {
-        *isValueInfoPreciseRef = false;
-    }
-
     if(!desiredValueType.IsDefinite())
     {
         return desiredValueType;
     }
 
-    if((instr->GetSrc1() && !IsPrepassSrcValueInfoPrecise(instr->GetSrc1(), src1Value)) ||
-       (instr->GetSrc2() && !IsPrepassSrcValueInfoPrecise(instr->GetSrc2(), src2Value)))
+    if(!isValueInfoPrecise)
     {
         // If the desired value type is not precise, the value type of the destination is derived from the value types of the
         // sources. Since the value type of a source sym is not definite, the destination value type also cannot be definite.
@@ -5399,42 +5408,45 @@ GlobOpt::GetPrepassValueTypeForDst(
         return desiredValueType.ToLikely();
     }
 
-    if(isValueInfoPreciseRef)
-    {
-        // The produced value info is derived from the sources, which have precise value infos
-        *isValueInfoPreciseRef = true;
-    }
     return desiredValueType;
 }
 
 bool
-GlobOpt::IsPrepassSrcValueInfoPrecise(IR::Opnd *const src, Value *const srcValue) const
+GlobOpt::IsPrepassSrcValueInfoPrecise(IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool * isSafeToTransferInPrepass) const
+{
+    return
+        (!instr->GetSrc1() || IsPrepassSrcValueInfoPrecise(instr->GetSrc1(), src1Value, isSafeToTransferInPrepass)) &&
+        (!instr->GetSrc2() || IsPrepassSrcValueInfoPrecise(instr->GetSrc2(), src2Value, isSafeToTransferInPrepass));
+}
+
+bool
+GlobOpt::IsPrepassSrcValueInfoPrecise(IR::Opnd *const src, Value *const srcValue, bool * isSafeToTransferInPrepass) const
 {
     Assert(IsLoopPrePass());
     Assert(src);
 
-    if(!src->IsRegOpnd() || !srcValue)
+    if (isSafeToTransferInPrepass)
+    {
+        *isSafeToTransferInPrepass = false;
+    }
+
+    if (!src->IsRegOpnd() || !srcValue)
     {
         return false;
     }
 
     ValueInfo *const srcValueInfo = srcValue->GetValueInfo();
-    if(!srcValueInfo->IsDefinite())
+    bool isValueInfoDefinite = srcValueInfo->IsDefinite();
+
+    StackSym * srcSym = src->AsRegOpnd()->m_sym;
+
+    bool isSafeToTransfer = IsSafeToTransferInPrepass(srcSym, srcValueInfo);
+    if (isSafeToTransferInPrepass)
     {
-        return false;
+        *isSafeToTransferInPrepass = isSafeToTransfer;
     }
 
-    StackSym *srcSym = src->AsRegOpnd()->m_sym;
-    Assert(!srcSym->IsTypeSpec());
-    int32 intConstantValue;
-    return
-        srcSym->IsFromByteCodeConstantTable() ||
-        (
-            srcValueInfo->TryGetIntConstantValue(&intConstantValue) &&
-            !Js::TaggedInt::IsOverflow(intConstantValue) &&
-            GetTaggedIntConstantStackSym(intConstantValue) == srcSym
-        ) ||
-        !currentBlock->loop->regAlloc.liveOnBackEdgeSyms->Test(srcSym->m_id);
+    return isValueInfoDefinite && isSafeToTransfer;
 }
 
 bool
@@ -5471,7 +5483,8 @@ Value *GlobOpt::CreateDstUntransferredIntValue(
     bool isValueInfoPrecise;
     if(IsLoopPrePass())
     {
-        valueType = GetPrepassValueTypeForDst(valueType, instr, src1Value, src2Value, &isValueInfoPrecise);
+        isValueInfoPrecise = IsPrepassSrcValueInfoPrecise(instr, src1Value, src2Value);
+        valueType = GetPrepassValueTypeForDst(valueType, instr, src1Value, src2Value, isValueInfoPrecise);
     }
     else
     {
@@ -5502,7 +5515,7 @@ GlobOpt::CreateDstUntransferredValue(
     ValueType valueType(desiredValueType);
     if(IsLoopPrePass())
     {
-        valueType = GetPrepassValueTypeForDst(valueType, instr, src1Value, src2Value);
+        valueType = GetPrepassValueTypeForDst(valueType, instr, src1Value, src2Value, IsPrepassSrcValueInfoPrecise(instr, src1Value, src2Value));
     }
     return NewGenericValue(valueType, instr->GetDst());
 }
@@ -5629,8 +5642,24 @@ GlobOpt::ValueNumberTransferDstInPrepass(IR::Instr *const instr, Value *const sr
 
     // In prepass we are going to copy the value but with a different value number
     // for aggressive int type spec.
-    const ValueType valueType(GetPrepassValueTypeForDst(src1ValueInfo->Type(), instr, src1Val, nullptr, &isValueInfoPrecise));
-    if(isValueInfoPrecise || (valueType == src1ValueInfo->Type() && src1ValueInfo->IsGeneric()))
+    bool isSafeToTransferInPrepass = false;
+    isValueInfoPrecise = IsPrepassSrcValueInfoPrecise(instr, src1Val, nullptr, &isSafeToTransferInPrepass);
+    
+    const ValueType valueType(GetPrepassValueTypeForDst(src1ValueInfo->Type(), instr, src1Val, nullptr, isValueInfoPrecise));
+    if(isValueInfoPrecise || isSafeToTransferInPrepass)
+    {
+        Assert(valueType == src1ValueInfo->Type());
+        if (!PHASE_OFF1(Js::AVTInPrePassPhase))
+        {
+            dstVal = src1Val;
+        }
+        else
+        {
+            dstVal = CopyValue(src1Val);
+            TrackCopiedValueForKills(dstVal);
+        }
+    }
+    else if (valueType == src1ValueInfo->Type() && src1ValueInfo->IsGeneric()) // this else branch is probably not needed
     {
         Assert(valueType == src1ValueInfo->Type());
         dstVal = CopyValue(src1Val);
@@ -8144,7 +8173,8 @@ GlobOpt::TypeSpecializeIntDst(IR::Instr* instr, Js::OpCode originalOpCode, Value
     bool isValueInfoPrecise;
     if(IsLoopPrePass())
     {
-        valueType = GetPrepassValueTypeForDst(valueType, instr, src1Value, src2Value, &isValueInfoPrecise);
+        isValueInfoPrecise = IsPrepassSrcValueInfoPrecise(instr, src1Value, src2Value);
+        valueType = GetPrepassValueTypeForDst(valueType, instr, src1Value, src2Value, isValueInfoPrecise);
     }
     else
     {
@@ -9642,7 +9672,7 @@ LOutsideSwitch:
                 // calculation, we want to insert the Conv_bool after the whole compare instruction
                 // block.
                 IR::Instr *putAfter = instr;
-                while (putAfter->m_next && putAfter->m_next->m_opcode == Js::OpCode::ByteCodeUses)
+                while (putAfter->m_next && putAfter->m_next->IsByteCodeUsesInstrFor(instr))
                 {
                     putAfter = putAfter->m_next;
                 }
@@ -14215,7 +14245,6 @@ GlobOpt::OptIsInvariant(
         return false;
 
         // Usually not worth hoisting these
-    case Js::OpCode::LdStr:
     case Js::OpCode::Ld_A:
     case Js::OpCode::Ld_I4:
     case Js::OpCode::LdC_A_I4:

+ 3 - 2
lib/Backend/GlobOpt.h

@@ -561,8 +561,9 @@ private:
     bool                    AreFromSameBytecodeFunc(IR::RegOpnd const* src1, IR::RegOpnd const* dst) const;
     Value *                 ValueNumberDst(IR::Instr **pInstr, Value *src1Val, Value *src2Val);
     Value *                 ValueNumberLdElemDst(IR::Instr **pInstr, Value *srcVal);
-    ValueType               GetPrepassValueTypeForDst(const ValueType desiredValueType, IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool *const isValueInfoPreciseRef = nullptr) const;
-    bool                    IsPrepassSrcValueInfoPrecise(IR::Opnd *const src, Value *const srcValue) const;
+    ValueType               GetPrepassValueTypeForDst(const ValueType desiredValueType, IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool const isValueInfoPreciseRef = false) const;
+    bool                    IsPrepassSrcValueInfoPrecise(IR::Opnd *const src, Value *const srcValue, bool * canTransferValueNumberToDst = nullptr) const;
+    bool                    IsPrepassSrcValueInfoPrecise(IR::Instr *const instr, Value *const src1Value, Value *const src2Value, bool * canTransferValueNumberToDst = nullptr) const;
     bool                    IsSafeToTransferInPrepass(StackSym * const sym, ValueInfo *const srcValueInfo) const;
     Value *                 CreateDstUntransferredIntValue(const int32 min, const int32 max, IR::Instr *const instr, Value *const src1Value, Value *const src2Value);
     Value *                 CreateDstUntransferredValue(const ValueType desiredValueType, IR::Instr *const instr, Value *const src1Value, Value *const src2Value);

+ 1 - 1
lib/Backend/GlobOptBailOut.cpp

@@ -1074,7 +1074,7 @@ GlobOpt::ConvertToByteCodeUses(IR::Instr * instr)
     instr->Remove();
     if (byteCodeUsesInstr)
     {
-        byteCodeUsesInstr->Aggregate();
+        byteCodeUsesInstr->AggregateFollowingByteCodeUses();
     }
     return byteCodeUsesInstr;
 }

+ 6 - 1
lib/Backend/GlobOptFields.cpp

@@ -117,6 +117,11 @@ GlobOpt::DoFieldPRE(Loop *loop) const
         return true;
     }
 
+    if (this->func->HasProfileInfo() && this->func->GetReadOnlyProfileInfo()->IsFieldPREDisabled())
+    {
+        return false;
+    }
+
     return DoFieldOpts(loop);
 }
 
@@ -147,7 +152,7 @@ GlobOpt::KillLiveFields(StackSym * stackSym, BVSparse<JitArenaAllocator> * bv)
 
     // If the sym has no objectSymInfo, it must not represent an object and, hence, has no type sym or
     // property syms to kill.
-    if (!stackSym->HasObjectInfo())
+    if (!stackSym->HasObjectInfo() || stackSym->IsSingleDef())
     {
         return;
     }

+ 93 - 15
lib/Backend/IR.cpp

@@ -984,28 +984,69 @@ void ByteCodeUsesInstr::SetBV(BVSparse<JitArenaAllocator>* newbv)
 // a compare, but still need to generate them for bailouts. Without this, we cause
 // problems because we end up with an instruction losing atomicity in terms of its
 // bytecode use and generation lifetimes.
-void ByteCodeUsesInstr::Aggregate()
+void ByteCodeUsesInstr::AggregateFollowingByteCodeUses()
 {
     IR::Instr* scanner = this->m_next;
     while (scanner && scanner->m_opcode == Js::OpCode::ByteCodeUses && scanner->GetByteCodeOffset() == this->GetByteCodeOffset() && scanner->GetDst() == nullptr)
     {
         IR::ByteCodeUsesInstr* target = scanner->AsByteCodeUsesInstr();
-        Assert(this->m_func == target->m_func);
-        if (target->byteCodeUpwardExposedUsed)
+        this->Aggregate(target);
+        scanner = scanner->m_next;
+    }
+}
+
+void ByteCodeUsesInstr::AggregatePrecedingByteCodeUses()
+{
+    IR::Instr * instr = this->m_prev;
+    while (instr && this->CanAggregateByteCodeUsesAcrossInstr(instr))
+    {
+        if (instr->IsByteCodeUsesInstr())
         {
-            if (this->byteCodeUpwardExposedUsed)
-            {
-                this->byteCodeUpwardExposedUsed->Or(target->byteCodeUpwardExposedUsed);
-                JitAdelete(target->byteCodeUpwardExposedUsed->GetAllocator(), target->byteCodeUpwardExposedUsed);
-                target->byteCodeUpwardExposedUsed = nullptr;
-            }
-            else
-            {
-                this->byteCodeUpwardExposedUsed = target->byteCodeUpwardExposedUsed;
-                target->byteCodeUpwardExposedUsed = nullptr;
-            }
+            IR::ByteCodeUsesInstr* precedingByteCodeUsesInstr = instr->AsByteCodeUsesInstr();
+            this->Aggregate(precedingByteCodeUsesInstr);
+        }
+        instr = instr->m_prev;
+    }
+}
+
+void ByteCodeUsesInstr::Aggregate(ByteCodeUsesInstr * byteCodeUsesInstr)
+{
+    Assert(this->m_func == byteCodeUsesInstr->m_func);
+    if (byteCodeUsesInstr->byteCodeUpwardExposedUsed)
+    {
+        if (this->byteCodeUpwardExposedUsed)
+        {
+            this->byteCodeUpwardExposedUsed->Or(byteCodeUsesInstr->byteCodeUpwardExposedUsed);
+            JitAdelete(byteCodeUsesInstr->byteCodeUpwardExposedUsed->GetAllocator(), byteCodeUsesInstr->byteCodeUpwardExposedUsed);
+            byteCodeUsesInstr->byteCodeUpwardExposedUsed = nullptr;
+        }
+        else
+        {
+            this->byteCodeUpwardExposedUsed = byteCodeUsesInstr->byteCodeUpwardExposedUsed;
+            byteCodeUsesInstr->byteCodeUpwardExposedUsed = nullptr;
+        }
+    }
+}
+
+bool Instr::CanAggregateByteCodeUsesAcrossInstr(Instr * instr)
+{
+    if (instr->GetByteCodeOffset() != Js::Constants::NoByteCodeOffset)
+    {
+        return (instr->GetByteCodeOffset() == this->GetByteCodeOffset()) &&
+                    (instr->IsByteCodeUsesInstr() ||
+                     instr->m_opcode == Js::OpCode::ToVar);
+    }
+    else
+    {
+        if (instr->HasBailOutInfo())
+        {
+            return (instr->GetBailOutInfo()->bailOutOffset == this->GetByteCodeOffset()) &&
+                instr->m_opcode == Js::OpCode::BailOnNotObject;
+        }
+        else
+        {
+            return (instr->m_opcode == Js::OpCode::Ld_A || instr->m_opcode == Js::OpCode::Ld_I4) && instr->GetSrc1()->IsImmediateOpnd(); // could have been inserted by PreLowerCanonicalize
         }
-        scanner = scanner->m_next;
     }
 }
 
@@ -2838,6 +2879,27 @@ IR::Instr *Instr::GetInsertBeforeByteCodeUsesInstr()
     return insertBeforeInstr;
 }
 
+IR::ByteCodeUsesInstr *
+Instr::GetFirstByteCodeUsesInstrBackward()
+{
+    IR::Instr * prevInstr = this->m_prev;
+    while (prevInstr && this->CanAggregateByteCodeUsesAcrossInstr(prevInstr))
+    {
+        if (prevInstr->IsByteCodeUsesInstr())
+        {
+            return prevInstr->AsByteCodeUsesInstr();
+        }
+        prevInstr = prevInstr->m_prev;
+    }
+    return nullptr;
+}
+
+bool
+Instr::IsByteCodeUsesInstrFor(IR::Instr * instr) const
+{
+    return this->IsByteCodeUsesInstr() && this->GetByteCodeOffset() == instr->GetByteCodeOffset();
+}
+
 ///----------------------------------------------------------------------------
 ///
 /// Instr::GetOrCreateContinueLabel
@@ -3341,6 +3403,22 @@ void Instr::Move(IR::Instr* insertInstr)
     insertInstr->InsertBefore(this);
 }
 
+void Instr::AggregateByteCodeUses()
+{
+    // Currently, this only aggregates byteCodeUpwardExposedUsed of those ByteCodeUses instructions 
+    // associated with this instr which have at most a ToVar, Ld_A, Ld_I4, or a BailOnNotObject between them.
+    IR::ByteCodeUsesInstr * primaryByteCodeUsesInstr = this->GetFirstByteCodeUsesInstrBackward();
+    if (primaryByteCodeUsesInstr)
+    {
+        primaryByteCodeUsesInstr->AggregatePrecedingByteCodeUses();
+        if (primaryByteCodeUsesInstr != this->m_prev)
+        {
+            primaryByteCodeUsesInstr->Unlink();
+            this->InsertBefore(primaryByteCodeUsesInstr);
+        }
+    }
+}
+
 IR::Instr* Instr::GetBytecodeArgOutCapture()
 {
     Assert(this->m_opcode == Js::OpCode::ArgOut_A_Inline ||

+ 9 - 1
lib/Backend/IR.h

@@ -290,6 +290,8 @@ public:
     IR::Instr *     GetPrevRealInstr() const;
     IR::Instr *     GetPrevRealInstrOrLabel() const;
     IR::Instr *     GetInsertBeforeByteCodeUsesInstr();
+    IR::ByteCodeUsesInstr * GetFirstByteCodeUsesInstrBackward();
+    bool            IsByteCodeUsesInstrFor(IR::Instr * instr) const;
     IR::LabelInstr *GetOrCreateContinueLabel(const bool isHelper = false);
     static bool     HasSymUseSrc(StackSym *sym, IR::Opnd*);
     static bool     HasSymUseDst(StackSym *sym, IR::Opnd*);
@@ -297,6 +299,7 @@ public:
     static bool     HasSymUseInRange(StackSym *sym, Instr *instrBegin, Instr *instrEnd);
     RegOpnd *       FindRegDef(StackSym *sym);
     static Instr*   FindSingleDefInstr(Js::OpCode opCode, Opnd* src);
+    bool            CanAggregateByteCodeUsesAcrossInstr(IR::Instr * instr);
 
     BranchInstr *   ChangeCmCCToBranchInstr(LabelInstr *targetInstr);
     static void     MoveRangeAfter(Instr * instrStart, Instr * instrLast, Instr * instrAfter);
@@ -466,6 +469,7 @@ public:
     bool       UsesAllFields();
     void       MoveArgs(bool generateByteCodeCapture = false);
     void       Move(IR::Instr* insertInstr);
+    void       AggregateByteCodeUses();
 private:
     void            ClearNumber() { this->m_number = 0; }
     void            SetNumber(uint32 number);
@@ -571,7 +575,11 @@ public:
     // a compare, but still need to generate them for bailouts. Without this, we cause
     // problems because we end up with an instruction losing atomicity in terms of its
     // bytecode use and generation lifetimes.
-    void Aggregate();
+    void AggregateFollowingByteCodeUses();
+    void AggregatePrecedingByteCodeUses();
+
+private:
+    void Aggregate(ByteCodeUsesInstr * byteCodeUsesInstr);
 };
 
 class JitProfilingInstr : public Instr

+ 7 - 0
lib/Backend/JITTimeProfileInfo.cpp

@@ -172,6 +172,7 @@ JITTimeProfileInfo::InitializeJITProfileData(
     data->flags |= profileInfo->IsPowIntIntTypeSpecDisabled() ? Flags_disablePowIntIntTypeSpec : 0;
     data->flags |= profileInfo->IsTagCheckDisabled() ? Flags_disableTagCheck : 0;
     data->flags |= profileInfo->IsOptimizeTryFinallyDisabled() ? Flags_disableOptimizeTryFinally : 0;
+    data->flags |= profileInfo->IsFieldPREDisabled() ? Flags_disableFieldPRE : 0;
 }
 
 const Js::LdLenInfo *
@@ -541,6 +542,12 @@ JITTimeProfileInfo::IsOptimizeTryFinallyDisabled() const
     return TestFlag(Flags_disableOptimizeTryFinally);
 }
 
+bool
+JITTimeProfileInfo::IsFieldPREDisabled() const
+{
+    return TestFlag(Flags_disableFieldPRE);
+}
+
 bool
 JITTimeProfileInfo::HasLdFldCallSiteInfo() const
 {

+ 3 - 1
lib/Backend/JITTimeProfileInfo.h

@@ -68,6 +68,7 @@ public:
     bool IsPowIntIntTypeSpecDisabled() const;
     bool IsTagCheckDisabled() const;
     bool IsOptimizeTryFinallyDisabled() const;
+    bool IsFieldPREDisabled() const;
 
 private:
     enum ProfileDataFlags : int64
@@ -109,7 +110,8 @@ private:
         Flags_disableLoopImplicitCallInfo = 1ll << 33,
         Flags_disablePowIntIntTypeSpec = 1ll << 34,
         Flags_disableTagCheck = 1ll << 35,
-        Flags_disableOptimizeTryFinally = 1ll << 36
+        Flags_disableOptimizeTryFinally = 1ll << 36,
+        Flags_disableFieldPRE = 1ll << 37
     };
 
     Js::ProfileId GetProfiledArrayCallSiteCount() const;

+ 5 - 1
lib/Backend/Lower.cpp

@@ -13495,7 +13495,7 @@ Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::L
                 indexOpnd, IR::IntConstOpnd::New(bailOutInfo->polymorphicCacheIndex, TyUint32, this->m_func), instr, false);
         }
 
-        if (bailOutInfo->bailOutRecord->GetType() == BailOutRecord::BailoutRecordType::Shared)
+        if (bailOutInfo->bailOutRecord->IsShared())
         {
             IR::Opnd *functionBodyOpnd;
             if (this->m_func->IsOOPJIT())
@@ -13629,6 +13629,10 @@ Lowerer::GenerateBailOut(IR::Instr * instr, IR::BranchInstr * branchInstr, IR::L
         {
             bailOutRecord = NativeCodeDataNewZ(this->m_func->GetNativeCodeDataAllocator(),
                 SharedBailOutRecord, bailOutInfo->bailOutOffset, bailOutInfo->polymorphicCacheIndex, instr->GetBailOutKind(), bailOutInfo->bailOutFunc);
+            if (bailOutInfo->isLoopTopBailOutInfo)
+            {
+                bailOutRecord->SetType(BailOutRecord::BailoutRecordType::SharedForLoopTop);
+            }
         }
         else
         {

+ 1 - 0
lib/Common/ConfigFlagsList.h

@@ -118,6 +118,7 @@ PHASE(All)
             PHASE(Forward)
                 PHASE(ValueTable)
                 PHASE(ValueNumbering)
+                PHASE(AVTInPrePass)
                 PHASE(PathDependentValues)
                     PHASE(TrackRelativeIntBounds)
                         PHASE(BoundCheckElimination)

+ 3 - 1
lib/Runtime/Language/DynamicProfileInfo.cpp

@@ -1868,6 +1868,7 @@ namespace Js
                 _u(" disableStackArgOpt : %s\n")
                 _u(" disableTagCheck : %s\n")
                 _u(" disableOptimizeTryFinally : %s\n"),
+                _u(" disableFieldPRE : %s\n"),
                 IsTrueOrFalse(this->bits.disableAggressiveIntTypeSpec),
                 IsTrueOrFalse(this->bits.disableAggressiveIntTypeSpec_jitLoopBody),
                 IsTrueOrFalse(this->bits.disableAggressiveMulIntTypeSpec),
@@ -1904,7 +1905,8 @@ namespace Js
                 IsTrueOrFalse(this->bits.disablePowIntIntTypeSpec),
                 IsTrueOrFalse(this->bits.disableStackArgOpt),
                 IsTrueOrFalse(this->bits.disableTagCheck),
-                IsTrueOrFalse(this->bits.disableOptimizeTryFinally));
+                IsTrueOrFalse(this->bits.disableOptimizeTryFinally),
+                IsTrueOrFalse(this->bits.disableFieldPRE));
         }
     }
 

+ 3 - 0
lib/Runtime/Language/DynamicProfileInfo.h

@@ -593,6 +593,7 @@ namespace Js
             Field(bool) disableStackArgOpt : 1;
             Field(bool) disableTagCheck : 1;
             Field(bool) disableOptimizeTryFinally : 1;
+            Field(bool) disableFieldPRE : 1;
         };
         Field(Bits) bits;
 
@@ -898,6 +899,8 @@ namespace Js
         void DisableTagCheck() { this->bits.disableTagCheck = true; }
         bool IsOptimizeTryFinallyDisabled() const { return bits.disableOptimizeTryFinally; }
         void DisableOptimizeTryFinally() { this->bits.disableOptimizeTryFinally = true; }
+        bool IsFieldPREDisabled() const { return bits.disableFieldPRE; }
+        void DisableFieldPRE() { this->bits.disableFieldPRE = true; }
 
         static bool IsCallSiteNoInfo(Js::LocalFunctionId functionId) { return functionId == CallSiteNoInfo; }
         int IncRejitCount() { return this->rejitCount++; }