diff --git a/lib/Backend/BackwardPass.cpp b/lib/Backend/BackwardPass.cpp index 9b4ce1658b8..13b0310d783 100644 --- a/lib/Backend/BackwardPass.cpp +++ b/lib/Backend/BackwardPass.cpp @@ -2261,7 +2261,7 @@ BackwardPass::DeadStoreTypeCheckBailOut(IR::Instr * instr) // By default, do not do this for stores, as it makes the presence of type checks unpredictable in the forward pass. // For instance, we can't predict which stores may cause reallocation of aux slots. - if (!PHASE_ON(Js::DeadStoreTypeChecksOnStoresPhase, this->func) && instr->GetDst() && instr->GetDst()->IsSymOpnd()) + if (instr->GetDst() && instr->GetDst()->IsSymOpnd()) { return; } diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index 57d506b57ef..8de311567ee 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -2190,27 +2190,46 @@ GlobOpt::CollectMemOpInfo(IR::Instr *instrBegin, IR::Instr *instr, Value *src1Va return false; } break; - case Js::OpCode::Decr_A: - isIncr = false; - case Js::OpCode::Incr_A: - isChangedByOne = true; - goto MemOpCheckInductionVariable; case Js::OpCode::Sub_I4: - case Js::OpCode::Sub_A: isIncr = false; - case Js::OpCode::Add_A: case Js::OpCode::Add_I4: { -MemOpCheckInductionVariable: - StackSym *sym = instr->GetSrc1()->GetStackSym(); - if (!sym) + // The only case in which these OpCodes can contribute to an inductionVariableChangeInfo + // is when the induction variable is being modified and overwritten aswell (ex: j = j + 1) + // and not when the induction variable is modified but not overwritten (ex: k = j + 1). + // This can either be detected in IR as + // s1 = Add_I4 s1 1 // Case #1, can be seen with "j++". + // or as + // s4(s2) = Add_I4 s3(s1) 1 // Case #2, can be see with "j = j + 1". + // s1 = Ld_A s2 + bool isInductionVar = false; + IR::Instr* nextInstr = instr->m_next; + if ( + // Checks for Case #1 and Case #2 + instr->GetDst()->GetStackSym() != nullptr && + instr->GetDst()->IsRegOpnd() && + ( + // Checks for Case #1 + (instr->GetDst()->GetStackSym() == instr->GetSrc1()->GetStackSym()) || + + // Checks for Case #2 + (nextInstr&& nextInstr->m_opcode == Js::OpCode::Ld_A && + nextInstr->GetSrc1()->IsRegOpnd() && + nextInstr->GetDst()->IsRegOpnd() && + GetVarSymID(instr->GetDst()->GetStackSym()) == nextInstr->GetSrc1()->GetStackSym()->m_id && + GetVarSymID(instr->GetSrc1()->GetStackSym()) == nextInstr->GetDst()->GetStackSym()->m_id) + ) + ) { - sym = instr->GetSrc2()->GetStackSym(); + isInductionVar = true; } + + // Even if dstIsInductionVar then dst == src1 so it's safe to use src1 as the induction sym always. + StackSym* sym = instr->GetSrc1()->GetStackSym(); SymID inductionSymID = GetVarSymID(sym); - if (IsSymIDInductionVariable(inductionSymID, this->currentBlock->loop)) + if (isInductionVar && IsSymIDInductionVariable(inductionSymID, this->currentBlock->loop)) { if (!isChangedByOne) { @@ -2290,7 +2309,6 @@ GlobOpt::CollectMemOpInfo(IR::Instr *instrBegin, IR::Instr *instr, Value *src1Va { inductionVariableChangeInfo.unroll++; } - inductionVariableChangeInfo.isIncremental = isIncr; loop->memOpInfo->inductionVariableChangeInfoMap->Item(inductionSymID, inductionVariableChangeInfo); if (sym->m_id != inductionSymID) @@ -2333,6 +2351,27 @@ GlobOpt::CollectMemOpInfo(IR::Instr *instrBegin, IR::Instr *instr, Value *src1Va } } NEXT_INSTR_IN_RANGE; + IR::Instr* prevInstr = instr->m_prev; + + // If an instr where the dst is an induction variable (and thus is being written to) is not caught by a case in the above + // switch statement (which implies that this instr does not contributes to a inductionVariableChangeInfo) and in the default + // case does not set doMemOp to false (which implies that this instr does not invalidate this MemOp), then FailFast as we + // should not be performing a MemOp under these conditions. + AssertOrFailFast(!instr->GetDst() || instr->m_opcode == Js::OpCode::IncrLoopBodyCount || !loop->memOpInfo || + + // Refer to "Case #2" described above in this function. For the following IR: + // Line #1: s4(s2) = Add_I4 s3(s1) 1 + // Line #2: s3(s1) = Ld_A s4(s2) + // do not consider line #2 as a violating instr + (instr->m_opcode == Js::OpCode::Ld_I4 && + prevInstr && (prevInstr->m_opcode == Js::OpCode::Add_I4 || prevInstr->m_opcode == Js::OpCode::Sub_I4) && + instr->GetSrc1()->IsRegOpnd() && + instr->GetDst()->IsRegOpnd() && + prevInstr->GetDst()->IsRegOpnd() && + instr->GetDst()->GetStackSym() == prevInstr->GetSrc1()->GetStackSym() && + instr->GetSrc1()->GetStackSym() == prevInstr->GetDst()->GetStackSym()) || + + !loop->memOpInfo->inductionVariableChangeInfoMap->ContainsKey(GetVarSymID(instr->GetDst()->GetStackSym()))); } return true; @@ -3619,7 +3658,7 @@ GlobOpt::OptSrc(IR::Opnd *opnd, IR::Instr * *pInstr, Value **indirIndexValRef, I opnd->SetValueType(valueType); - if(!IsLoopPrePass() && opnd->IsSymOpnd() && valueType.IsDefinite()) + if(!IsLoopPrePass() && opnd->IsSymOpnd() && (valueType.IsDefinite() || valueType.IsNotTaggedValue())) { if (opnd->AsSymOpnd()->m_sym->IsPropertySym()) { diff --git a/lib/Backend/GlobOpt.h b/lib/Backend/GlobOpt.h index c457c82e463..8bae2a93bb8 100644 --- a/lib/Backend/GlobOpt.h +++ b/lib/Backend/GlobOpt.h @@ -366,6 +366,7 @@ class JsArrayKills (valueType.IsArrayOrObjectWithArray() && ( (killsArraysWithNoMissingValues && valueType.HasNoMissingValues()) || + (killsObjectArraysWithNoMissingValues && !valueType.IsArray() && valueType.HasNoMissingValues()) || (killsNativeArrays && !valueType.HasVarElements()) ) ); diff --git a/lib/Backend/GlobOptFields.cpp b/lib/Backend/GlobOptFields.cpp index 88bf72d3255..ee8cf829604 100644 --- a/lib/Backend/GlobOptFields.cpp +++ b/lib/Backend/GlobOptFields.cpp @@ -905,7 +905,7 @@ GlobOpt::FinishOptPropOp(IR::Instr *instr, IR::PropertySymOpnd *opnd, BasicBlock SymID opndId = opnd->HasObjectTypeSym() ? opnd->GetObjectTypeSym()->m_id : -1; - if (!isObjTypeChecked) + if (!isObjTypeSpecialized || opnd->IsBeingAdded()) { if (block->globOptData.maybeWrittenTypeSyms == nullptr) { @@ -1154,6 +1154,19 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd Assert(opnd->IsTypeCheckSeqCandidate()); Assert(opnd->HasObjectTypeSym()); + if (opnd->HasTypeMismatch()) + { + if (emitsTypeCheckOut != nullptr) + { + *emitsTypeCheckOut = false; + } + if (changesTypeValueOut != nullptr) + { + *changesTypeValueOut = false; + } + return false; + } + bool isStore = opnd == instr->GetDst(); bool isTypeDead = opnd->IsTypeDead(); bool consumeType = makeChanges && !IsLoopPrePass(); @@ -1261,7 +1274,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd // a new type value here. isSpecialized = false; - if (consumeType) + if (makeChanges) { opnd->SetTypeMismatch(true); } @@ -1305,7 +1318,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd // a new type value here. isSpecialized = false; - if (consumeType) + if (makeChanges) { opnd->SetTypeMismatch(true); } @@ -1356,7 +1369,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd { // Indicates failure/mismatch isSpecialized = false; - if (consumeType) + if (makeChanges) { opnd->SetTypeMismatch(true); } @@ -1456,7 +1469,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd // a new type value here. isSpecialized = false; - if (consumeType) + if (makeChanges) { opnd->SetTypeMismatch(true); } diff --git a/lib/Backend/Lower.cpp b/lib/Backend/Lower.cpp index 6079830a5bc..22dc8b7492e 100644 --- a/lib/Backend/Lower.cpp +++ b/lib/Backend/Lower.cpp @@ -7427,9 +7427,6 @@ Lowerer::GenerateStFldWithCachedType(IR::Instr *instrStFld, bool* continueAsHelp if (hasTypeCheckBailout) { - AssertMsg(PHASE_ON1(Js::ObjTypeSpecIsolatedFldOpsWithBailOutPhase) || !PHASE_ON(Js::DeadStoreTypeChecksOnStoresPhase, this->m_func) || !propertySymOpnd->IsTypeDead() || propertySymOpnd->TypeCheckRequired(), - "Why does a field store have a type check bailout, if its type is dead?"); - if (instrStFld->GetBailOutInfo()->bailOutInstr != instrStFld) { // Set the cache index in the bailout info so that the generated code will write it into the @@ -7489,7 +7486,7 @@ Lowerer::GenerateCachedTypeCheck(IR::Instr *instrChk, IR::PropertySymOpnd *prope // cache and no type check bailout. In the latter case, we can wind up doing expensive failed equivalence checks // repeatedly and never rejit. bool doEquivTypeCheck = - instrChk->HasEquivalentTypeCheckBailOut() || + (instrChk->HasEquivalentTypeCheckBailOut() && (propertySymOpnd->TypeCheckRequired() || propertySymOpnd == instrChk->GetDst())) || (propertySymOpnd->HasEquivalentTypeSet() && !(propertySymOpnd->HasFinalType() && propertySymOpnd->HasInitialType()) && !propertySymOpnd->MustDoMonoCheck() && diff --git a/test/fieldopts/OS23440664.js b/test/fieldopts/OS23440664.js new file mode 100644 index 00000000000..34438f1aeea --- /dev/null +++ b/test/fieldopts/OS23440664.js @@ -0,0 +1,17 @@ +//Reduced Switches: -printsystemexception -maxinterpretcount:1 -maxsimplejitruncount:1 -werexceptionsupport -oopjit- -bvt -off:bailonnoprofile -force:fixdataprops -forcejitloopbody +var shouldBailout = false; +var IntArr0 = []; +function test0() { + var loopInvariant = shouldBailout; + function makeArrayLength() { + return Math.floor(); + } + makeArrayLength(); + makeArrayLength(); + prop0 = 1; + Object; + for (; shouldBailout ? (Object()) : (IntArr0[Object & 1] = '') ? Object : 0;) { + } +} +test0(); +WScript.Echo('pass'); diff --git a/test/fieldopts/rlexe.xml b/test/fieldopts/rlexe.xml index 6e4023368ba..dcee05a0d10 100644 --- a/test/fieldopts/rlexe.xml +++ b/test/fieldopts/rlexe.xml @@ -857,4 +857,10 @@ argobjlengthhoist.js + + + OS23440664.js + -off:bailonnoprofile -force:fixdataprops -forcejitloopbody + +