Bug 1462353 - Remove new.target slot from generators, clean up generator code a bit. r=arai
authorJan de Mooij <jdemooij@mozilla.com>
Mon, 21 May 2018 09:13:05 +0200
changeset 419122 fb1dfccf693f2032ba2f26c77a2f8c6bd64c27ca
parent 419121 01d9634bf82ef07002dd46be45e11ffc677a4167
child 419123 daca961e0469a2786bdfabce6b03b4840f6b8ee9
push id34026
push userapavel@mozilla.com
push dateMon, 21 May 2018 09:47:33 +0000
treeherdermozilla-central@dc1868d255be [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1462353
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1462353 - Remove new.target slot from generators, clean up generator code a bit. r=arai
js/src/jit-test/tests/generators/bug1462353.js
js/src/jit/BaselineCompiler.cpp
js/src/vm/GeneratorObject.cpp
js/src/vm/GeneratorObject.h
js/src/vm/Stack-inl.h
js/src/vm/Stack.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/generators/bug1462353.js
@@ -0,0 +1,11 @@
+class Base {}
+class Derived extends Base {
+    constructor() {
+        var fun = async() => {
+            for (var i = 0; i < 20; i++) {} // Trigger OSR.
+            super();
+        };
+        fun();
+    }
+}
+d = new Derived();
--- a/js/src/jit/BaselineCompiler.cpp
+++ b/js/src/jit/BaselineCompiler.cpp
@@ -4693,29 +4693,16 @@ BaselineCompiler::emit_JSOP_RESUME()
     masm.loadPtr(Address(scratch1, JSScript::offsetOfBaselineScript()), scratch1);
     masm.branchPtr(Assembler::BelowOrEqual, scratch1, ImmPtr(BASELINE_DISABLED_SCRIPT), &interpret);
 
 #ifdef JS_TRACE_LOGGING
     if (!emitTraceLoggerResume(scratch1, regs))
         return false;
 #endif
 
-    Register constructing = regs.takeAny();
-    ValueOperand newTarget = regs.takeAnyValue();
-    masm.loadValue(Address(genObj, GeneratorObject::offsetOfNewTargetSlot()), newTarget);
-    masm.move32(Imm32(0), constructing);
-    {
-        Label notConstructing;
-        masm.branchTestObject(Assembler::NotEqual, newTarget, &notConstructing);
-        masm.pushValue(newTarget);
-        masm.move32(Imm32(CalleeToken_FunctionConstructing), constructing);
-        masm.bind(&notConstructing);
-    }
-    regs.add(newTarget);
-
     // Push |undefined| for all formals.
     Register scratch2 = regs.takeAny();
     Label loop, loopDone;
     masm.load16ZeroExtend(Address(callee, JSFunction::offsetOfNargs()), scratch2);
     masm.bind(&loop);
     masm.branchTest32(Assembler::Zero, scratch2, scratch2, &loopDone);
     {
         masm.pushValue(UndefinedValue());
@@ -4730,24 +4717,20 @@ BaselineCompiler::emit_JSOP_RESUME()
     // Update BaselineFrame frameSize field and create the frame descriptor.
     masm.computeEffectiveAddress(Address(BaselineFrameReg, BaselineFrame::FramePointerOffset),
                                  scratch2);
     masm.subStackPtrFrom(scratch2);
     masm.store32(scratch2, Address(BaselineFrameReg, BaselineFrame::reverseOffsetOfFrameSize()));
     masm.makeFrameDescriptor(scratch2, JitFrame_BaselineJS, JitFrameLayout::Size());
 
     masm.Push(Imm32(0)); // actual argc
-
-    // Duplicate PushCalleeToken with a variable instead.
-    masm.orPtr(constructing, callee);
-    masm.Push(callee);
+    masm.PushCalleeToken(callee, /* constructing = */ false);
     masm.Push(scratch2); // frame descriptor
 
     regs.add(callee);
-    regs.add(constructing);
 
     // Load the return value.
     ValueOperand retVal = regs.takeAnyValue();
     masm.loadValue(frame.addressOfStackValue(frame.peek(-1)), retVal);
 
     // Push a fake return address on the stack. We will resume here when the
     // generator returns.
     Label genStart, returnTarget;
--- a/js/src/vm/GeneratorObject.cpp
+++ b/js/src/vm/GeneratorObject.cpp
@@ -16,16 +16,17 @@
 
 using namespace js;
 
 JSObject*
 GeneratorObject::create(JSContext* cx, AbstractFramePtr frame)
 {
     MOZ_ASSERT(frame.script()->isGenerator() || frame.script()->isAsync());
     MOZ_ASSERT(frame.script()->nfixed() == 0);
+    MOZ_ASSERT(!frame.isConstructing());
 
     Rooted<GlobalObject*> global(cx, cx->global());
 
     RootedValue pval(cx);
     RootedObject fun(cx, frame.callee());
     // FIXME: This would be faster if we could avoid doing a lookup to get
     // the prototype for the instance.  Bug 906600.
     if (!GetProperty(cx, fun, fun, cx->names().prototype, &pval))
@@ -36,17 +37,16 @@ GeneratorObject::create(JSContext* cx, A
         if (!proto)
             return nullptr;
     }
     Rooted<GeneratorObject*> genObj(cx, NewObjectWithGivenProto<GeneratorObject>(cx, proto));
     if (!genObj)
         return nullptr;
 
     genObj->setCallee(*frame.callee());
-    genObj->setNewTarget(frame.newTarget());
     genObj->setEnvironmentChain(*frame.environmentChain());
     if (frame.script()->needsArgsObj())
         genObj->setArgsObj(frame.argsObj());
     genObj->clearExpressionStack();
 
     return genObj;
 }
 
@@ -132,19 +132,18 @@ js::GeneratorThrowOrReturn(JSContext* cx
 bool
 GeneratorObject::resume(JSContext* cx, InterpreterActivation& activation,
                         HandleObject obj, HandleValue arg, GeneratorObject::ResumeKind resumeKind)
 {
     Rooted<GeneratorObject*> genObj(cx, &obj->as<GeneratorObject>());
     MOZ_ASSERT(genObj->isSuspended());
 
     RootedFunction callee(cx, &genObj->callee());
-    RootedValue newTarget(cx, genObj->newTarget());
     RootedObject envChain(cx, &genObj->environmentChain());
-    if (!activation.resumeGeneratorFrame(callee, newTarget, envChain))
+    if (!activation.resumeGeneratorFrame(callee, envChain))
         return false;
     activation.regs().fp()->setResumedGenerator();
 
     if (genObj->hasArgsObj())
         activation.regs().fp()->initArgsObj(genObj->argsObj());
 
     if (genObj->hasExpressionStack() && !genObj->isExpressionStackEmpty()) {
         uint32_t len = genObj->expressionStack().getDenseInitializedLength();
--- a/js/src/vm/GeneratorObject.h
+++ b/js/src/vm/GeneratorObject.h
@@ -24,17 +24,16 @@ class GeneratorObject : public NativeObj
     static const int32_t YIELD_AND_AWAIT_INDEX_CLOSING = INT32_MAX - 1;
 
     enum {
         CALLEE_SLOT = 0,
         ENV_CHAIN_SLOT,
         ARGS_OBJ_SLOT,
         EXPRESSION_STACK_SLOT,
         YIELD_AND_AWAIT_INDEX_SLOT,
-        NEWTARGET_SLOT,
         RESERVED_SLOTS
     };
 
     enum ResumeKind { NEXT, THROW, RETURN };
 
     static const Class class_;
 
   private:
@@ -109,27 +108,16 @@ class GeneratorObject : public NativeObj
     }
     void setExpressionStack(ArrayObject& expressionStack) {
         setFixedSlot(EXPRESSION_STACK_SLOT, ObjectValue(expressionStack));
     }
     void clearExpressionStack() {
         setFixedSlot(EXPRESSION_STACK_SLOT, NullValue());
     }
 
-    bool isConstructing() const {
-        return getFixedSlot(NEWTARGET_SLOT).isObject();
-    }
-    const Value& newTarget() const {
-        return getFixedSlot(NEWTARGET_SLOT);
-    }
-    void setNewTarget(const Value& newTarget) {
-        setFixedSlot(NEWTARGET_SLOT, newTarget);
-    }
-
-
     // The yield index slot is abused for a few purposes.  It's undefined if
     // it hasn't been set yet (before the initial yield), and null if the
     // generator is closed. If the generator is running, the yield index is
     // YIELD_AND_AWAIT_INDEX_RUNNING. If the generator is in that bizarre
     // "closing" state, the yield index is YIELD_AND_AWAIT_INDEX_CLOSING.
     //
     // If the generator is suspended, it's the yield index (stored as
     // JSOP_INITIALYIELD/JSOP_YIELD/JSOP_AWAIT operand) of the yield
@@ -175,17 +163,16 @@ class GeneratorObject : public NativeObj
         return getFixedSlot(CALLEE_SLOT).isNull();
     }
     void setClosed() {
         setFixedSlot(CALLEE_SLOT, NullValue());
         setFixedSlot(ENV_CHAIN_SLOT, NullValue());
         setFixedSlot(ARGS_OBJ_SLOT, NullValue());
         setFixedSlot(EXPRESSION_STACK_SLOT, NullValue());
         setFixedSlot(YIELD_AND_AWAIT_INDEX_SLOT, NullValue());
-        setFixedSlot(NEWTARGET_SLOT, NullValue());
     }
 
     bool isAfterYield();
     bool isAfterAwait();
 
   private:
     bool isAfterYieldOrAwait(JSOp op);
 
@@ -200,19 +187,16 @@ class GeneratorObject : public NativeObj
         return getFixedSlotOffset(ARGS_OBJ_SLOT);
     }
     static size_t offsetOfYieldAndAwaitIndexSlot() {
         return getFixedSlotOffset(YIELD_AND_AWAIT_INDEX_SLOT);
     }
     static size_t offsetOfExpressionStackSlot() {
         return getFixedSlotOffset(EXPRESSION_STACK_SLOT);
     }
-    static size_t offsetOfNewTargetSlot() {
-        return getFixedSlotOffset(NEWTARGET_SLOT);
-    }
 };
 
 bool GeneratorThrowOrReturn(JSContext* cx, AbstractFramePtr frame, Handle<GeneratorObject*> obj,
                             HandleValue val, uint32_t resumeKind);
 void SetGeneratorClosed(JSContext* cx, AbstractFramePtr frame);
 
 MOZ_MUST_USE bool
 CheckGeneratorResumptionValue(JSContext* cx, HandleValue v);
--- a/js/src/vm/Stack-inl.h
+++ b/js/src/vm/Stack-inl.h
@@ -313,50 +313,48 @@ InterpreterStack::pushInlineFrame(JSCont
                       constructing);
 
     regs.prepareToRun(*fp, script);
     return true;
 }
 
 MOZ_ALWAYS_INLINE bool
 InterpreterStack::resumeGeneratorCallFrame(JSContext* cx, InterpreterRegs& regs,
-                                           HandleFunction callee, HandleValue newTarget,
-                                           HandleObject envChain)
+                                           HandleFunction callee, HandleObject envChain)
 {
     MOZ_ASSERT(callee->isGenerator() || callee->isAsync());
     RootedScript script(cx, JSFunction::getOrCreateScript(cx, callee));
     InterpreterFrame* prev = regs.fp();
     jsbytecode* prevpc = regs.pc;
     Value* prevsp = regs.sp;
     MOZ_ASSERT(prev);
 
     script->ensureNonLazyCanonicalFunction();
 
     LifoAlloc::Mark mark = allocator_.mark();
 
-    MaybeConstruct constructing = MaybeConstruct(newTarget.isObject());
+    // (Async) generators and async functions are not constructors.
+    MOZ_ASSERT(!callee->isConstructor());
 
     // Include callee, |this|, and maybe |new.target|
     unsigned nformal = callee->nargs();
-    unsigned nvals = 2 + constructing + nformal + script->nslots();
+    unsigned nvals = 2 + nformal + script->nslots();
 
     uint8_t* buffer = allocateFrame(cx, sizeof(InterpreterFrame) + nvals * sizeof(Value));
     if (!buffer)
         return false;
 
     Value* argv = reinterpret_cast<Value*>(buffer) + 2;
     argv[-2] = ObjectValue(*callee);
     argv[-1] = UndefinedValue();
     SetValueRangeToUndefined(argv, nformal);
-    if (constructing)
-        argv[nformal] = newTarget;
 
-    InterpreterFrame* fp = reinterpret_cast<InterpreterFrame*>(argv + nformal + constructing);
+    InterpreterFrame* fp = reinterpret_cast<InterpreterFrame*>(argv + nformal);
     fp->mark_ = mark;
-    fp->initCallFrame(prev, prevpc, prevsp, *callee, script, argv, 0, constructing);
+    fp->initCallFrame(prev, prevpc, prevsp, *callee, script, argv, 0, NO_CONSTRUCT);
     fp->resumeGeneratorFrame(envChain);
 
     regs.prepareToRun(*fp, script);
     return true;
 }
 
 MOZ_ALWAYS_INLINE void
 InterpreterStack::popInlineFrame(InterpreterRegs& regs)
@@ -643,16 +641,28 @@ AbstractFramePtr::unsetIsDebuggee()
         asBaselineFrame()->unsetIsDebuggee();
     else if (isWasmDebugFrame())
         asWasmDebugFrame()->unsetIsDebuggee();
     else
         asRematerializedFrame()->unsetIsDebuggee();
 }
 
 inline bool
+AbstractFramePtr::isConstructing() const
+{
+    if (isInterpreterFrame())
+        return asInterpreterFrame()->isConstructing();
+    if (isBaselineFrame())
+        return asBaselineFrame()->isConstructing();
+    if (isRematerializedFrame())
+        return asRematerializedFrame()->isConstructing();
+    MOZ_CRASH("Unexpected frame");
+}
+
+inline bool
 AbstractFramePtr::hasArgs() const
 {
     return isFunctionFrame();
 }
 
 inline bool
 AbstractFramePtr::hasScript() const
 {
@@ -958,21 +968,20 @@ InterpreterActivation::popInlineFrame(In
     (void)frame; // Quell compiler warning.
     MOZ_ASSERT(regs_.fp() == frame);
     MOZ_ASSERT(regs_.fp() != entryFrame_);
 
     cx_->interpreterStack().popInlineFrame(regs_);
 }
 
 inline bool
-InterpreterActivation::resumeGeneratorFrame(HandleFunction callee, HandleValue newTarget,
-                                            HandleObject envChain)
+InterpreterActivation::resumeGeneratorFrame(HandleFunction callee, HandleObject envChain)
 {
     InterpreterStack& stack = cx_->interpreterStack();
-    if (!stack.resumeGeneratorCallFrame(cx_, regs_, callee, newTarget, envChain))
+    if (!stack.resumeGeneratorCallFrame(cx_, regs_, callee, envChain))
         return false;
 
     MOZ_ASSERT(regs_.fp()->script()->compartment() == compartment_);
     return true;
 }
 
 /* static */ inline mozilla::Maybe<LiveSavedFrameCache::FramePtr>
 LiveSavedFrameCache::FramePtr::create(const FrameIter& iter)
--- a/js/src/vm/Stack.h
+++ b/js/src/vm/Stack.h
@@ -226,16 +226,17 @@ class AbstractFramePtr
     inline bool hasScript() const;
     inline JSScript* script() const;
     inline wasm::Instance* wasmInstance() const;
     inline GlobalObject* global() const;
     inline JSFunction* callee() const;
     inline Value calleev() const;
     inline Value& thisArgument() const;
 
+    inline bool isConstructing() const;
     inline Value newTarget() const;
 
     inline bool debuggerNeedsCheckPrimitiveReturn() const;
 
     inline bool isFunctionFrame() const;
     inline bool isNonStrictDirectEvalFrame() const;
     inline bool isStrictEvalFrame() const;
 
@@ -901,18 +902,17 @@ class InterpreterStack
     // The interpreter can push light-weight, "inline" frames without entering a
     // new InterpreterActivation or recursively calling Interpret.
     bool pushInlineFrame(JSContext* cx, InterpreterRegs& regs, const CallArgs& args,
                          HandleScript script, MaybeConstruct constructing);
 
     void popInlineFrame(InterpreterRegs& regs);
 
     bool resumeGeneratorCallFrame(JSContext* cx, InterpreterRegs& regs,
-                                  HandleFunction callee, HandleValue newTarget,
-                                  HandleObject envChain);
+                                  HandleFunction callee, HandleObject envChain);
 
     inline void purge(JSRuntime* rt);
 
     size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const {
         return allocator_.sizeOfExcludingThis(mallocSizeOf);
     }
 };
 
@@ -1547,18 +1547,17 @@ class InterpreterActivation : public Act
   public:
     inline InterpreterActivation(RunState& state, JSContext* cx, InterpreterFrame* entryFrame);
     inline ~InterpreterActivation();
 
     inline bool pushInlineFrame(const CallArgs& args, HandleScript script,
                                 MaybeConstruct constructing);
     inline void popInlineFrame(InterpreterFrame* frame);
 
-    inline bool resumeGeneratorFrame(HandleFunction callee, HandleValue newTarget,
-                                     HandleObject envChain);
+    inline bool resumeGeneratorFrame(HandleFunction callee, HandleObject envChain);
 
     InterpreterFrame* current() const {
         return regs_.fp();
     }
     InterpreterRegs& regs() {
         return regs_;
     }
     InterpreterFrame* entryFrame() const {