Bug 1548075: Remove 'closing' state from AbstractGeneratorObject. r=iain
authorJim Blandy <jimb@mozilla.com>
Tue, 07 May 2019 18:04:17 +0000
changeset 534824 32a3534e33fd7b59dd6759cc605c78b4663f16d5
parent 534823 6c0d9bd9f398bcff4da0dc15a49a87d060dff9be
child 534825 3f601f3cf988a2a63a4b3e54bf9e5deb49b36309
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersiain
bugs1548075
milestone68.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 1548075: Remove 'closing' state from AbstractGeneratorObject. r=iain The generator 'closing' state, indicated by the value RESUME_INDEX_CLOSING in an AbstractGeneratorObject's RESUME_INDEX_SLOT is never distinguished from the 'running' state, so RESUME_INDEX_CLOSING, and its associated setters and predicates, can be removed. RESUME_INDEX_RUNNING remains as a magic value appearing in the resume index slot, and is now used as an upper bound on normal resume index values instead of RESUME_INDEX_CLOSING. Differential Revision: https://phabricator.services.mozilla.com/D29391
js/src/frontend/BytecodeEmitter.cpp
js/src/jit/CacheIRCompiler.cpp
js/src/vm/Debugger.cpp
js/src/vm/GeneratorObject.cpp
js/src/vm/GeneratorObject.h
js/src/vm/SelfHosting.cpp
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -2253,17 +2253,17 @@ bool BytecodeEmitter::isRunOnceLambda() 
          !funbox->isAsync() && !funbox->function()->explicitName();
 }
 
 bool BytecodeEmitter::allocateResumeIndex(ptrdiff_t offset,
                                           uint32_t* resumeIndex) {
   static constexpr uint32_t MaxResumeIndex = JS_BITMASK(24);
 
   static_assert(
-      MaxResumeIndex < uint32_t(AbstractGeneratorObject::RESUME_INDEX_CLOSING),
+      MaxResumeIndex < uint32_t(AbstractGeneratorObject::RESUME_INDEX_RUNNING),
       "resumeIndex should not include magic AbstractGeneratorObject "
       "resumeIndex values");
   static_assert(
       MaxResumeIndex <= INT32_MAX / sizeof(uintptr_t),
       "resumeIndex * sizeof(uintptr_t) must fit in an int32. JIT code relies "
       "on this when loading resume entries from BaselineScript");
 
   *resumeIndex = bytecodeSection().resumeOffsetList().length();
--- a/js/src/jit/CacheIRCompiler.cpp
+++ b/js/src/jit/CacheIRCompiler.cpp
@@ -4353,23 +4353,23 @@ bool CacheIRCompiler::emitCallIsSuspende
   masm.branchTestObject(Assembler::NotEqual, input, &returnFalse);
 
   // Test if it's a GeneratorObject.
   masm.unboxObject(input, scratch);
   masm.branchTestObjClass(Assembler::NotEqual, scratch,
                           &GeneratorObject::class_, scratch2, scratch,
                           &returnFalse);
 
-  // If the resumeIndex slot holds an int32 value < RESUME_INDEX_CLOSING,
+  // If the resumeIndex slot holds an int32 value < RESUME_INDEX_RUNNING,
   // the generator is suspended.
   Address addr(scratch, AbstractGeneratorObject::offsetOfResumeIndexSlot());
   masm.branchTestInt32(Assembler::NotEqual, addr, &returnFalse);
   masm.unboxInt32(addr, scratch);
   masm.branch32(Assembler::AboveOrEqual, scratch,
-                Imm32(AbstractGeneratorObject::RESUME_INDEX_CLOSING),
+                Imm32(AbstractGeneratorObject::RESUME_INDEX_RUNNING),
                 &returnFalse);
 
   masm.moveValue(BooleanValue(true), output.valueReg());
   masm.jump(&done);
 
   masm.bind(&returnFalse);
   masm.moveValue(BooleanValue(false), output.valueReg());
 
--- a/js/src/vm/Debugger.cpp
+++ b/js/src/vm/Debugger.cpp
@@ -2057,17 +2057,17 @@ ResumeMode Debugger::fireEnterFrame(JSCo
   RootedValue scriptFrame(cx);
 
   FrameIter iter(cx);
 
 #if DEBUG
   // Assert that the hook won't be able to re-enter the generator.
   if (iter.hasScript() && *iter.pc() == JSOP_AFTERYIELD) {
     auto* genObj = GetGeneratorObjectForFrame(cx, iter.abstractFramePtr());
-    MOZ_ASSERT(genObj->isRunning() || genObj->isClosing());
+    MOZ_ASSERT(genObj->isRunning());
   }
 #endif
 
   Maybe<AutoRealm> ar;
   ar.emplace(cx, object);
 
   if (!getFrame(cx, iter, &scriptFrame)) {
     return reportUncaughtException(ar);
--- a/js/src/vm/GeneratorObject.cpp
+++ b/js/src/vm/GeneratorObject.cpp
@@ -95,17 +95,17 @@ bool AbstractGeneratorObject::suspend(JS
     genObj->setExpressionStack(*stack);
   }
 
   return true;
 }
 
 void AbstractGeneratorObject::finalSuspend(HandleObject obj) {
   auto* genObj = &obj->as<AbstractGeneratorObject>();
-  MOZ_ASSERT(genObj->isRunning() || genObj->isClosing());
+  MOZ_ASSERT(genObj->isRunning());
   genObj->setClosed();
 }
 
 AbstractGeneratorObject* js::GetGeneratorObjectForFrame(
     JSContext* cx, AbstractFramePtr frame) {
   cx->check(frame);
   MOZ_ASSERT(frame.isGeneratorFrame());
 
@@ -134,27 +134,27 @@ void js::SetGeneratorClosed(JSContext* c
       callObj.getSlot(shape->slot()).toObject().as<AbstractGeneratorObject>();
   genObj.setClosed();
 }
 
 bool js::GeneratorThrowOrReturn(JSContext* cx, AbstractFramePtr frame,
                                 Handle<AbstractGeneratorObject*> genObj,
                                 HandleValue arg,
                                 GeneratorResumeKind resumeKind) {
+  MOZ_ASSERT(genObj->isRunning());
   if (resumeKind == GeneratorResumeKind::Throw) {
     cx->setPendingExceptionAndCaptureStack(arg);
   } else {
     MOZ_ASSERT(resumeKind == GeneratorResumeKind::Return);
 
     MOZ_ASSERT_IF(genObj->is<GeneratorObject>(), arg.isObject());
     frame.setReturnValue(arg);
 
     RootedValue closing(cx, MagicValue(JS_GENERATOR_CLOSING));
     cx->setPendingException(closing, nullptr);
-    genObj->setClosing();
   }
   return false;
 }
 
 bool AbstractGeneratorObject::resume(JSContext* cx,
                                      InterpreterActivation& activation,
                                      Handle<AbstractGeneratorObject*> genObj,
                                      HandleValue arg) {
@@ -304,17 +304,17 @@ bool AbstractGeneratorObject::isAfterYie
   return isAfterYieldOrAwait(JSOP_YIELD);
 }
 
 bool AbstractGeneratorObject::isAfterAwait() {
   return isAfterYieldOrAwait(JSOP_AWAIT);
 }
 
 bool AbstractGeneratorObject::isAfterYieldOrAwait(JSOp op) {
-  if (isClosed() || isClosing() || isRunning()) {
+  if (isClosed() || isRunning()) {
     return false;
   }
 
   JSScript* script = callee().nonLazyScript();
   jsbytecode* code = script->code();
   uint32_t nextOffset = script->resumeOffsets()[resumeIndex()];
   if (code[nextOffset] != JSOP_AFTERYIELD) {
     return false;
--- a/js/src/vm/GeneratorObject.h
+++ b/js/src/vm/GeneratorObject.h
@@ -15,20 +15,19 @@
 #include "vm/Stack.h"
 
 namespace js {
 
 enum class GeneratorResumeKind { Next, Throw, Return };
 
 class AbstractGeneratorObject : public NativeObject {
  public:
-  // Magic values stored in the resumeIndex slot when the generator is
+  // Magic value stored in the resumeIndex slot when the generator is
   // running or closing. See the resumeIndex comment below.
   static const int32_t RESUME_INDEX_RUNNING = INT32_MAX;
-  static const int32_t RESUME_INDEX_CLOSING = INT32_MAX - 1;
 
   enum {
     CALLEE_SLOT = 0,
     ENV_CHAIN_SLOT,
     ARGS_OBJ_SLOT,
     EXPRESSION_STACK_SLOT,
     RESUME_INDEX_SLOT,
     RESERVED_SLOTS
@@ -111,59 +110,49 @@ class AbstractGeneratorObject : public N
   }
   void clearExpressionStack() {
     setFixedSlot(EXPRESSION_STACK_SLOT, NullValue());
   }
 
   // The resumeIndex 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 resumeIndex is
-  // RESUME_INDEX_RUNNING. If the generator is in that bizarre "closing"
-  // state, the resumeIndex is RESUME_INDEX_CLOSING.
+  // RESUME_INDEX_RUNNING.
   //
   // If the generator is suspended, it's the resumeIndex (stored as
   // JSOP_INITIALYIELD/JSOP_YIELD/JSOP_AWAIT operand) of the yield instruction
   // that suspended the generator. The resumeIndex can be mapped to the
   // bytecode offset (interpreter) or to the native code offset (JIT).
 
   bool isBeforeInitialYield() const {
     return getFixedSlot(RESUME_INDEX_SLOT).isUndefined();
   }
   bool isRunning() const {
     return getFixedSlot(RESUME_INDEX_SLOT) == Int32Value(RESUME_INDEX_RUNNING);
   }
-  bool isClosing() const {
-    return getFixedSlot(RESUME_INDEX_SLOT) == Int32Value(RESUME_INDEX_CLOSING);
-  }
   bool isSuspended() const {
     // Note: also update Baseline's IsSuspendedGenerator code if this
     // changes.
-    static_assert(RESUME_INDEX_CLOSING < RESUME_INDEX_RUNNING,
-                  "test below should return false for RESUME_INDEX_RUNNING");
     Value resumeIndex = getFixedSlot(RESUME_INDEX_SLOT);
-    return resumeIndex.isInt32() && resumeIndex.toInt32() < RESUME_INDEX_CLOSING;
+    return resumeIndex.isInt32() && resumeIndex.toInt32() < RESUME_INDEX_RUNNING;
   }
   void setRunning() {
     MOZ_ASSERT(isSuspended());
     setFixedSlot(RESUME_INDEX_SLOT, Int32Value(RESUME_INDEX_RUNNING));
   }
-  void setClosing() {
-    MOZ_ASSERT(isRunning());
-    setFixedSlot(RESUME_INDEX_SLOT, Int32Value(RESUME_INDEX_CLOSING));
-  }
   void setResumeIndex(jsbytecode* pc) {
     MOZ_ASSERT(*pc == JSOP_INITIALYIELD || *pc == JSOP_YIELD ||
                *pc == JSOP_AWAIT);
 
     MOZ_ASSERT_IF(JSOp(*pc) == JSOP_INITIALYIELD,
                   getFixedSlot(RESUME_INDEX_SLOT).isUndefined());
-    MOZ_ASSERT_IF(JSOp(*pc) != JSOP_INITIALYIELD, isRunning() || isClosing());
+    MOZ_ASSERT_IF(JSOp(*pc) != JSOP_INITIALYIELD, isRunning());
 
     uint32_t resumeIndex = GET_UINT24(pc);
-    MOZ_ASSERT(resumeIndex < uint32_t(RESUME_INDEX_CLOSING));
+    MOZ_ASSERT(resumeIndex < uint32_t(RESUME_INDEX_RUNNING));
 
     setFixedSlot(RESUME_INDEX_SLOT, Int32Value(resumeIndex));
     MOZ_ASSERT(isSuspended());
   }
   void setResumeIndex(int32_t resumeIndex) {
     setFixedSlot(RESUME_INDEX_SLOT, Int32Value(resumeIndex));
   }
   uint32_t resumeIndex() const {
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -961,17 +961,17 @@ bool js::intrinsic_IsSuspendedGenerator(
 
 static bool intrinsic_GeneratorIsRunning(JSContext* cx, unsigned argc,
                                          Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   MOZ_ASSERT(args.length() == 1);
   MOZ_ASSERT(args[0].isObject());
 
   GeneratorObject* genObj = &args[0].toObject().as<GeneratorObject>();
-  args.rval().setBoolean(genObj->isRunning() || genObj->isClosing());
+  args.rval().setBoolean(genObj->isRunning());
   return true;
 }
 
 static bool intrinsic_GeneratorSetClosed(JSContext* cx, unsigned argc,
                                          Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
   MOZ_ASSERT(args.length() == 1);
   MOZ_ASSERT(args[0].isObject());