From c2b0d896d76f2838c46fc80ff10f199d92b58576 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Fri, 20 Nov 2020 10:52:52 +0000 Subject: [PATCH] Bug 1673553 part 32 - Remove ArgumentCheck bailout code. r=iain This also gets rid of LStart and the unused entrySnapshot_ field. Differential Revision: https://phabricator.services.mozilla.com/D97577 --- js/src/jit/BaselineBailouts.cpp | 144 ++++++++++++-------------------- js/src/jit/CodeGenerator.cpp | 5 -- js/src/jit/IonTypes.h | 6 -- js/src/jit/LIR.cpp | 1 - js/src/jit/LIR.h | 12 +-- js/src/jit/Lowering.cpp | 12 +-- js/src/jit/MIR.h | 8 +- js/src/jit/shared/LIR-shared.h | 10 --- 8 files changed, 58 insertions(+), 140 deletions(-) diff --git a/js/src/jit/BaselineBailouts.cpp b/js/src/jit/BaselineBailouts.cpp index db60e2ddeca5..af2abe24852c 100644 --- a/js/src/jit/BaselineBailouts.cpp +++ b/js/src/jit/BaselineBailouts.cpp @@ -604,115 +604,84 @@ bool BaselineStackBuilder::buildBaselineFrame() { flags |= BaselineFrame::DEBUGGEE; } + // Get |envChain|. JSObject* envChain = nullptr; - Value returnValue = UndefinedValue(); - ArgumentsObject* argsObj = nullptr; + Value envChainSlot = iter_.read(); + if (envChainSlot.isObject()) { + // The env slot has been updated from UndefinedValue. It must be the + // complete initial environment. + envChain = &envChainSlot.toObject(); - BailoutKind bailoutKind = iter_.bailoutKind(); - if (bailoutKind == BailoutKind::ArgumentCheck) { - // We may fail before the envChain slot is set. Skip it and use - // the function's initial environment. This will be fixed up - // later if needed in |FinishBailoutToBaseline|, which calls - // |EnsureHasEnvironmentObjects|. - // TODO(no-TI): remove ArgumentCheck kind. - JitSpew(JitSpew_BaselineBailouts, - " BailoutKind::ArgumentCheck! Using function's environment"); - envChain = fun_->environment(); - - // Skip envChain. - iter_.skip(); - - // Skip return value. - iter_.skip(); - - // Scripts with |argumentsHasVarBinding| have an extra slot. - // Skip argsObj if present. - if (script_->argumentsHasVarBinding()) { - JitSpew(JitSpew_BaselineBailouts, - " BailoutKind::ArgumentCheck for script with " - "argumentsHasVarBinding! " - "Using empty arguments object"); - iter_.skip(); + // Set the HAS_INITIAL_ENV flag if needed. + if (fun_ && fun_->needsFunctionEnvironmentObjects()) { + MOZ_ASSERT(fun_->nonLazyScript()->initialEnvironmentShape()); + MOZ_ASSERT(!fun_->needsExtraBodyVarEnvironment()); + flags |= BaselineFrame::HAS_INITIAL_ENV; } } else { - // Get |envChain|. - Value envChainSlot = iter_.read(); - if (envChainSlot.isObject()) { - // The env slot has been updated from UndefinedValue. It must be the - // complete initial environment. - envChain = &envChainSlot.toObject(); + MOZ_ASSERT(envChainSlot.isUndefined() || + envChainSlot.isMagic(JS_OPTIMIZED_OUT)); + MOZ_ASSERT(envChainSlotCanBeOptimized()); - // Set the HAS_INITIAL_ENV flag if needed. - if (fun_ && fun_->needsFunctionEnvironmentObjects()) { - MOZ_ASSERT(fun_->nonLazyScript()->initialEnvironmentShape()); - MOZ_ASSERT(!fun_->needsExtraBodyVarEnvironment()); - flags |= BaselineFrame::HAS_INITIAL_ENV; - } + // The env slot has been optimized out. + // Get it from the function or script. + if (fun_) { + envChain = fun_->environment(); + } else if (script_->module()) { + envChain = script_->module()->environment(); } else { - MOZ_ASSERT(envChainSlot.isUndefined() || - envChainSlot.isMagic(JS_OPTIMIZED_OUT)); - MOZ_ASSERT(envChainSlotCanBeOptimized()); - - // The env slot has been optimized out. - // Get it from the function or script. - if (fun_) { - envChain = fun_->environment(); - } else if (script_->module()) { - envChain = script_->module()->environment(); - } else { - // For global scripts without a non-syntactic env the env - // chain is the script's global lexical environment. (We do - // not compile scripts with a non-syntactic global scope). - // Also note that it's invalid to resume into the prologue in - // this case because the prologue expects the env chain in R1 - // for eval and global scripts. - MOZ_ASSERT(!script_->isForEval()); - MOZ_ASSERT(!script_->hasNonSyntacticScope()); - envChain = &(script_->global().lexicalEnvironment()); - } - } - - // Get |returnValue| if present. - if (script_->noScriptRval()) { - // Don't use the return value (likely a JS_OPTIMIZED_OUT MagicValue) to - // not confuse Baseline. - iter_.skip(); - } else { - returnValue = iter_.read(); - flags |= BaselineFrame::HAS_RVAL; - } - - // Get |argsObj| if present. - if (script_->argumentsHasVarBinding()) { - Value maybeArgsObj = iter_.read(); - MOZ_ASSERT(maybeArgsObj.isObject() || maybeArgsObj.isUndefined() || - maybeArgsObj.isMagic(JS_OPTIMIZED_OUT)); - if (maybeArgsObj.isObject()) { - argsObj = &maybeArgsObj.toObject().as(); - } + // For global scripts without a non-syntactic env the env + // chain is the script's global lexical environment. (We do + // not compile scripts with a non-syntactic global scope). + // Also note that it's invalid to resume into the prologue in + // this case because the prologue expects the env chain in R1 + // for eval and global scripts. + MOZ_ASSERT(!script_->isForEval()); + MOZ_ASSERT(!script_->hasNonSyntacticScope()); + envChain = &(script_->global().lexicalEnvironment()); } } - MOZ_ASSERT(envChain); // Write |envChain|. + MOZ_ASSERT(envChain); JitSpew(JitSpew_BaselineBailouts, " EnvChain=%p", envChain); blFrame()->setEnvironmentChain(envChain); + // Get |returnValue| if present. + Value returnValue = UndefinedValue(); + if (script_->noScriptRval()) { + // Don't use the return value (likely a JS_OPTIMIZED_OUT MagicValue) to + // not confuse Baseline. + iter_.skip(); + } else { + returnValue = iter_.read(); + flags |= BaselineFrame::HAS_RVAL; + } + // Write |returnValue|. JitSpew(JitSpew_BaselineBailouts, " ReturnValue=%016" PRIx64, *((uint64_t*)&returnValue)); blFrame()->setReturnValue(returnValue); + // Get |argsObj| if present. + ArgumentsObject* argsObj = nullptr; + if (script_->argumentsHasVarBinding()) { + Value maybeArgsObj = iter_.read(); + MOZ_ASSERT(maybeArgsObj.isObject() || maybeArgsObj.isUndefined() || + maybeArgsObj.isMagic(JS_OPTIMIZED_OUT)); + if (maybeArgsObj.isObject()) { + argsObj = &maybeArgsObj.toObject().as(); + } + } + // Note: we do not need to initialize the scratchValue field in BaselineFrame. // Write |flags|. blFrame()->setFlags(flags); // Write |icScript|. - if (JitOptions.warpBuilder) { - JitSpew(JitSpew_BaselineBailouts, " ICScript=%p", icScript_); - blFrame()->setICScript(icScript_); - } + JitSpew(JitSpew_BaselineBailouts, " ICScript=%p", icScript_); + blFrame()->setICScript(icScript_); // initArgsObjUnchecked modifies the frame's flags, so call it after setFlags. if (argsObj) { @@ -2185,11 +2154,6 @@ bool jit::FinishBailoutToBaseline(BaselineBailoutInfo* bailoutInfoArg) { JSScript::argumentsOptimizationFailed(cx, innerScript); break; - case BailoutKind::ArgumentCheck: - // Do nothing, bailout will resume before the argument monitor ICs. - // This is unreachable in Warp. - MOZ_ASSERT(!JitOptions.warpBuilder); - break; case BailoutKind::BoundsCheck: HandleBoundsCheckFailure(cx, outerScript, innerScript); break; diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 296b76c61a28..d5a208710106 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -3787,8 +3787,6 @@ void CodeGenerator::visitIsConstructing(LIsConstructing* lir) { masm.andPtr(Imm32(0x1), output); } -void CodeGenerator::visitStart(LStart* lir) {} - void CodeGenerator::visitReturn(LReturn* lir) { #if defined(JS_NUNBOX32) DebugOnly type = lir->getOperand(TYPE_INDEX); @@ -6879,9 +6877,6 @@ void CodeGenerator::emitDebugForceBailing(LInstruction* lir) { if (!lir->snapshot()) { return; } - if (lir->isStart()) { - return; - } if (lir->isOsiPoint()) { return; } diff --git a/js/src/jit/IonTypes.h b/js/src/jit/IonTypes.h index 0a4545d3c4a5..39827539d811 100644 --- a/js/src/jit/IonTypes.h +++ b/js/src/jit/IonTypes.h @@ -198,10 +198,6 @@ enum class BailoutKind : uint8_t { // END Invalid assumptions bailouts - // A bailout at the very start of a function indicates that there may be - // a type mismatch in the arguments that necessitates a reflow. - ArgumentCheck, - // A bailout triggered by a bounds-check failure. BoundsCheck, @@ -363,8 +359,6 @@ inline const char* BailoutKindString(BailoutKind kind) { return "DoubleOutput"; // Other bailouts. - case BailoutKind::ArgumentCheck: - return "ArgumentCheck"; case BailoutKind::BoundsCheck: return "BoundsCheck"; case BailoutKind::ShapeGuard: diff --git a/js/src/jit/LIR.cpp b/js/src/jit/LIR.cpp index 9555ac7a79bb..a9fd9d3162dc 100644 --- a/js/src/jit/LIR.cpp +++ b/js/src/jit/LIR.cpp @@ -29,7 +29,6 @@ LIRGraph::LIRGraph(MIRGraph* mir) numInstructions_(1), // First id is 1. localSlotCount_(0), argumentSlotCount_(0), - entrySnapshot_(nullptr), mir_(*mir) {} bool LIRGraph::addConstantToPool(const Value& v, uint32_t* index) { diff --git a/js/src/jit/LIR.h b/js/src/jit/LIR.h index e59808839ae0..1bf9607f9a84 100644 --- a/js/src/jit/LIR.h +++ b/js/src/jit/LIR.h @@ -1840,9 +1840,6 @@ class LIRGraph { // Number of stack slots needed for argument construction for calls. uint32_t argumentSlotCount_; - // Snapshot taken before any LIR has been lowered. - LSnapshot* entrySnapshot_; - MIRGraph& mir_; public: @@ -1898,14 +1895,7 @@ class LIRGraph { MOZ_MUST_USE bool addConstantToPool(const Value& v, uint32_t* index); size_t numConstants() const { return constantPool_.length(); } Value* constantPool() { return &constantPool_[0]; } - void setEntrySnapshot(LSnapshot* snapshot) { - MOZ_ASSERT(!entrySnapshot_); - entrySnapshot_ = snapshot; - } - LSnapshot* entrySnapshot() const { - MOZ_ASSERT(entrySnapshot_); - return entrySnapshot_; - } + bool noteNeedsSafepoint(LInstruction* ins); size_t numNonCallSafepoints() const { return nonCallSafepoints_.length(); } LInstruction* getNonCallSafepoint(size_t i) const { diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index a7d2871a037c..d155d39217fc 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -1994,17 +1994,7 @@ void LIRGenerator::visitStringConvertCase(MStringConvertCase* ins) { assignSafepoint(lir, ins); } -void LIRGenerator::visitStart(MStart* start) { - LStart* lir = new (alloc()) LStart; - - // Create a snapshot that captures the initial state of the function. - assignSnapshot(lir, start->bailoutKind()); - if (start->block()->graph().entryBlock() == start->block()) { - lirGraph_.setEntrySnapshot(lir->snapshot()); - } - - add(lir); -} +void LIRGenerator::visitStart(MStart* start) {} void LIRGenerator::visitNop(MNop* nop) {} diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index d517a68898c9..8081e33fd415 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -1379,13 +1379,9 @@ class MVariadicT : public T { using MVariadicInstruction = MVariadicT; -// Generates an LSnapshot without further effect. +// TODO(no-TI): try to remove this instruction. class MStart : public MNullaryInstruction { - MStart() : MNullaryInstruction(classOpcode) { - // The snapshot for this instruction covers bailouts that happen before - // the env chain is set. See BaselineStackBuilder::buildBaselineFrame. - setBailoutKind(BailoutKind::ArgumentCheck); - } + MStart() : MNullaryInstruction(classOpcode) {} public: INSTRUCTION_HEADER(Start) diff --git a/js/src/jit/shared/LIR-shared.h b/js/src/jit/shared/LIR-shared.h index cdac47ade54a..53f0a61ccb7c 100644 --- a/js/src/jit/shared/LIR-shared.h +++ b/js/src/jit/shared/LIR-shared.h @@ -3393,16 +3393,6 @@ class LPowHalfD : public LInstructionHelper<1, 1, 0> { MPowHalf* mir() const { return mir_->toPowHalf(); } }; -// No-op instruction that is used to hold the entry snapshot. This simplifies -// register allocation as it doesn't need to sniff the snapshot out of the -// LIRGraph. -class LStart : public LInstructionHelper<0, 0, 0> { - public: - LIR_HEADER(Start) - - LStart() : LInstructionHelper(classOpcode) {} -}; - class LNaNToZero : public LInstructionHelper<1, 1, 1> { public: LIR_HEADER(NaNToZero)