From 8f8d409007b82d643215a1e57edea365d6485e5e Mon Sep 17 00:00:00 2001 From: Mariusz Kierski Date: Sun, 17 Jul 2016 10:22:33 +0900 Subject: [PATCH] Bug 1185106 - Part 0.1: Refactor JSOP_DEFFUN. r=efaust,jandem,till,h4writer MozReview-Commit-ID: 8XpAiHEzWVm --- js/src/frontend/BytecodeEmitter.cpp | 11 ++++++++++- js/src/jit/BaselineCompiler.cpp | 9 ++++----- js/src/jit/CodeGenerator.cpp | 3 ++- js/src/jit/IonBuilder.cpp | 6 +----- js/src/jit/Lowering.cpp | 6 +++++- js/src/jit/MIR.h | 19 +++++-------------- js/src/jit/shared/LIR-shared.h | 12 ++++++++---- js/src/vm/Interpreter.cpp | 24 +++--------------------- js/src/vm/Opcodes.h | 6 +++--- 9 files changed, 41 insertions(+), 55 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 1d51666a99b4..37223938e0d5 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -6942,6 +6942,13 @@ BytecodeEmitter::emitFunction(ParseNode* pn, bool needsProto) MOZ_ASSERT(pn->getOp() == JSOP_FUNWITHPROTO || pn->getOp() == JSOP_LAMBDA); pn->setOp(JSOP_FUNWITHPROTO); } + + if (pn->getOp() == JSOP_DEFFUN) { + if (!emitIndex32(JSOP_LAMBDA, index)) + return false; + return emit1(JSOP_DEFFUN); + } + return emitIndex32(pn->getOp(), index); } @@ -6972,7 +6979,9 @@ BytecodeEmitter::emitFunction(ParseNode* pn, bool needsProto) MOZ_ASSERT(sc->isGlobalContext() || sc->isEvalContext()); MOZ_ASSERT(pn->getOp() == JSOP_NOP); switchToPrologue(); - if (!emitIndex32(JSOP_DEFFUN, index)) + if (!emitIndex32(JSOP_LAMBDA, index)) + return false; + if (!emit1(JSOP_DEFFUN)) return false; if (!updateSourceCoordNotes(pn->pn_pos.begin)) return false; diff --git a/js/src/jit/BaselineCompiler.cpp b/js/src/jit/BaselineCompiler.cpp index 846a6cd3c816..5e2f9a4a0edc 100644 --- a/js/src/jit/BaselineCompiler.cpp +++ b/js/src/jit/BaselineCompiler.cpp @@ -2750,15 +2750,14 @@ static const VMFunction DefFunOperationInfo = bool BaselineCompiler::emit_JSOP_DEFFUN() { - RootedFunction fun(cx, script->getFunction(GET_UINT32_INDEX(pc))); - - frame.syncStack(0); - masm.loadPtr(frame.addressOfEnvironmentChain(), R0.scratchReg()); + frame.popRegsAndSync(1); + masm.unboxObject(R0, R0.scratchReg()); + masm.loadPtr(frame.addressOfEnvironmentChain(), R1.scratchReg()); prepareVMCall(); - pushArg(ImmGCPtr(fun)); pushArg(R0.scratchReg()); + pushArg(R1.scratchReg()); pushArg(ImmGCPtr(script)); return callVM(DefFunOperationInfo); diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 9e55d5bb42e3..922260b2e06f 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -4694,7 +4694,8 @@ CodeGenerator::visitDefFun(LDefFun* lir) { Register envChain = ToRegister(lir->environmentChain()); - pushArg(ImmGCPtr(lir->mir()->fun())); + Register fun = ToRegister(lir->fun()); + pushArg(fun); pushArg(envChain); pushArg(ImmGCPtr(current->mir()->info().script())); diff --git a/js/src/jit/IonBuilder.cpp b/js/src/jit/IonBuilder.cpp index e10eb5eea685..c8987acf8677 100644 --- a/js/src/jit/IonBuilder.cpp +++ b/js/src/jit/IonBuilder.cpp @@ -13442,13 +13442,9 @@ IonBuilder::jsop_deflexical(uint32_t index) bool IonBuilder::jsop_deffun(uint32_t index) { - JSFunction* fun = script()->getFunction(index); - if (IsAsmJSModule(fun)) - return abort("asm.js module function"); - MOZ_ASSERT(analysis().usesEnvironmentChain()); - MDefFun* deffun = MDefFun::New(alloc(), fun, current->environmentChain()); + MDefFun* deffun = MDefFun::New(alloc(), current->pop(), current->environmentChain()); current->add(deffun); return resumeAfter(deffun); diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index efe86356204f..4a04a4e7a202 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -207,7 +207,11 @@ LIRGenerator::visitDefLexical(MDefLexical* ins) void LIRGenerator::visitDefFun(MDefFun* ins) { - LDefFun* lir = new(alloc()) LDefFun(useRegisterAtStart(ins->environmentChain())); + MDefinition* fun = ins->fun(); + MOZ_ASSERT(fun->type() == MIRType::Object); + + LDefFun* lir = new(alloc()) LDefFun(useRegisterAtStart(fun), + useRegisterAtStart(ins->environmentChain())); add(lir, ins); assignSafepoint(lir, ins); } diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index 50136ff7a87b..9be745c24ef9 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -8106,31 +8106,22 @@ class MDefLexical }; class MDefFun - : public MUnaryInstruction, - public NoTypePolicy::Data + : public MBinaryInstruction, + public ObjectPolicy<0>::Data { - CompilerFunction fun_; - private: - MDefFun(JSFunction* fun, MDefinition* envChain) - : MUnaryInstruction(envChain), - fun_(fun) + MDefFun(MDefinition* fun, MDefinition* envChain) + : MBinaryInstruction(fun, envChain) {} public: INSTRUCTION_HEADER(DefFun) TRIVIAL_NEW_WRAPPERS - NAMED_OPERANDS((0, environmentChain)) + NAMED_OPERANDS((0, fun), (1, environmentChain)) - JSFunction* fun() const { - return fun_; - } bool possiblyCalls() const override { return true; } - bool appendRoots(MRootList& roots) const override { - return roots.append(fun_); - } }; class MRegExp : public MNullaryInstruction diff --git a/js/src/jit/shared/LIR-shared.h b/js/src/jit/shared/LIR-shared.h index ff346326bc23..8bd46a6b2bf4 100644 --- a/js/src/jit/shared/LIR-shared.h +++ b/js/src/jit/shared/LIR-shared.h @@ -1550,19 +1550,23 @@ class LDefLexical : public LCallInstructionHelper<0, 0, 0> } }; -class LDefFun : public LCallInstructionHelper<0, 1, 0> +class LDefFun : public LCallInstructionHelper<0, 2, 0> { public: LIR_HEADER(DefFun) - explicit LDefFun(const LAllocation& envChain) + LDefFun(const LAllocation& fun, const LAllocation& envChain) { - setOperand(0, envChain); + setOperand(0, fun); + setOperand(1, envChain); } - const LAllocation* environmentChain() { + const LAllocation* fun() { return getOperand(0); } + const LAllocation* environmentChain() { + return getOperand(1); + } MDefFun* mir() const { return mir_->toDefFun(); } diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index a460fc7fcdec..291e9b2de013 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -3446,9 +3446,10 @@ CASE(JSOP_DEFFUN) * a compound statement (not at the top statement level of global code, or * at the top level of a function body). */ - ReservedRooted fun(&rootFunction0, script->getFunction(GET_UINT32_INDEX(REGS.pc))); + ReservedRooted fun(&rootFunction0, ®S.sp[-1].toObject().as()); if (!DefFunOperation(cx, script, REGS.fp()->environmentChain(), fun)) goto error; + REGS.sp--; } END_CASE(JSOP_DEFFUN) @@ -4330,27 +4331,8 @@ js::LambdaArrow(JSContext* cx, HandleFunction fun, HandleObject parent, HandleVa bool js::DefFunOperation(JSContext* cx, HandleScript script, HandleObject envChain, - HandleFunction funArg) + HandleFunction fun) { - /* - * If static link is not current scope, clone fun's object to link to the - * current scope via parent. We do this to enable sharing of compiled - * functions among multiple equivalent scopes, amortizing the cost of - * compilation over a number of executions. Examples include XUL scripts - * and event handlers shared among Firefox or other Mozilla app chrome - * windows, and user-defined JS functions precompiled and then shared among - * requests in server-side JS. - */ - RootedFunction fun(cx, funArg); - if (fun->isNative() || fun->environment() != envChain) { - fun = CloneFunctionObjectIfNotSingleton(cx, fun, envChain, nullptr, TenuredObject); - if (!fun) - return false; - } else { - MOZ_ASSERT(script->treatAsRunOnce()); - MOZ_ASSERT(!script->functionNonDelazifying()); - } - /* * We define the function as a property of the variable object and not the * current scope chain even for the case of function expression statements diff --git a/js/src/vm/Opcodes.h b/js/src/vm/Opcodes.h index 6f2efa6409ca..38f351575c80 100644 --- a/js/src/vm/Opcodes.h +++ b/js/src/vm/Opcodes.h @@ -1300,10 +1300,10 @@ * scripts where use of dynamic scoping inhibits optimization. * Category: Variables and Scopes * Type: Variables - * Operands: uint32_t funcIndex - * Stack: => + * Operands: + * Stack: fun => */ \ - macro(JSOP_DEFFUN, 127,"deffun", NULL, 5, 0, 0, JOF_OBJECT) \ + macro(JSOP_DEFFUN, 127,"deffun", NULL, 1, 1, 0, JOF_BYTE) \ /* Defines the new constant binding on global lexical scope. * * Throws if a binding with the same name already exists on the scope, or