diff --git a/Build/NuGet/.pack-version b/Build/NuGet/.pack-version index dbb2469a538..3c3b474f3fc 100644 --- a/Build/NuGet/.pack-version +++ b/Build/NuGet/.pack-version @@ -1 +1 @@ -1.11.13 +1.11.14 diff --git a/lib/Backend/BackwardPass.cpp b/lib/Backend/BackwardPass.cpp index cd87cd04adc..af1908b2bf5 100644 --- a/lib/Backend/BackwardPass.cpp +++ b/lib/Backend/BackwardPass.cpp @@ -2195,6 +2195,13 @@ BackwardPass::DeadStoreTypeCheckBailOut(IR::Instr * instr) return; } + // 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 (instr->GetDst() && instr->GetDst()->IsSymOpnd()) + { + return; + } + IR::BailOutKind oldBailOutKind = instr->GetBailOutKind(); if (!IR::IsTypeCheckBailOutKind(oldBailOutKind)) { diff --git a/lib/Backend/GlobOpt.cpp b/lib/Backend/GlobOpt.cpp index e19eed15127..43b52dba7e9 100644 --- a/lib/Backend/GlobOpt.cpp +++ b/lib/Backend/GlobOpt.cpp @@ -2161,27 +2161,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) { @@ -2246,7 +2265,6 @@ GlobOpt::CollectMemOpInfo(IR::Instr *instrBegin, IR::Instr *instr, Value *src1Va { inductionVariableChangeInfo.unroll++; } - inductionVariableChangeInfo.isIncremental = isIncr; loop->memOpInfo->inductionVariableChangeInfoMap->Item(inductionSymID, inductionVariableChangeInfo); } @@ -2284,6 +2302,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; @@ -3564,7 +3603,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 e141d2c84db..725ac0ea975 100644 --- a/lib/Backend/GlobOpt.h +++ b/lib/Backend/GlobOpt.h @@ -370,6 +370,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 449a68d8d3d..e91c89f995e 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) { @@ -1122,6 +1122,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(); @@ -1229,7 +1242,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd // a new type value here. isSpecialized = false; - if (consumeType) + if (makeChanges) { opnd->SetTypeMismatch(true); } @@ -1273,7 +1286,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd // a new type value here. isSpecialized = false; - if (consumeType) + if (makeChanges) { opnd->SetTypeMismatch(true); } @@ -1324,7 +1337,7 @@ GlobOpt::ProcessPropOpInTypeCheckSeq(IR::Instr* instr, IR::PropertySymOpnd *opnd { // Indicates failure/mismatch isSpecialized = false; - if (consumeType) + if (makeChanges) { opnd->SetTypeMismatch(true); } @@ -1423,7 +1436,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 6424c657b69..a4f2bb45071 100644 --- a/lib/Backend/Lower.cpp +++ b/lib/Backend/Lower.cpp @@ -7420,9 +7420,6 @@ Lowerer::GenerateStFldWithCachedType(IR::Instr *instrStFld, bool* continueAsHelp if (hasTypeCheckBailout) { - AssertMsg(PHASE_ON1(Js::ObjTypeSpecIsolatedFldOpsWithBailOutPhase) || !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 @@ -7482,7 +7479,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() && propertySymOpnd->TypeCheckRequired()) || + (instrChk->HasEquivalentTypeCheckBailOut() && (propertySymOpnd->TypeCheckRequired() || propertySymOpnd == instrChk->GetDst())) || (propertySymOpnd->HasEquivalentTypeSet() && !(propertySymOpnd->HasFinalType() && propertySymOpnd->HasInitialType()) && !propertySymOpnd->MustDoMonoCheck() && diff --git a/lib/Common/ChakraCoreVersion.h b/lib/Common/ChakraCoreVersion.h index d2c83b5d830..3acf93ea50d 100644 --- a/lib/Common/ChakraCoreVersion.h +++ b/lib/Common/ChakraCoreVersion.h @@ -17,7 +17,7 @@ // ChakraCore version number definitions (used in ChakraCore binary metadata) #define CHAKRA_CORE_MAJOR_VERSION 1 #define CHAKRA_CORE_MINOR_VERSION 11 -#define CHAKRA_CORE_PATCH_VERSION 13 +#define CHAKRA_CORE_PATCH_VERSION 14 #define CHAKRA_CORE_VERSION_RELEASE_QFE 0 // Redundant with PATCH_VERSION. Keep this value set to 0. // ------------- 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 bcf581aab62..62a4a786678 100644 --- a/test/fieldopts/rlexe.xml +++ b/test/fieldopts/rlexe.xml @@ -852,4 +852,10 @@ argobjlengthhoist.js + + + OS23440664.js + -off:bailonnoprofile -force:fixdataprops -forcejitloopbody + +