Bug 976596 - Block-scoped variables indexing refactor r=luke
authorAndy Wingo <wingo@igalia.com>
Tue, 25 Feb 2014 18:46:15 +0100
changeset 170420 f0516583976066555ba08caf87e4f25af7455e18
parent 170419 6967361e6aeb085e5030074f856720b278225e4a
child 170421 a278b0807420dabbe59b6785a8fee2c734320f94
push id40228
push userwingo@igalia.com
push dateTue, 25 Feb 2014 17:46:48 +0000
treeherdermozilla-inbound@f05165839760 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs976596
milestone30.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 976596 - Block-scoped variables indexing refactor r=luke
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/Parser.cpp
js/src/jit/BaselineFrame.cpp
js/src/jit/CompileInfo.h
js/src/jsopcode.cpp
js/src/vm/ScopeObject.cpp
js/src/vm/ScopeObject.h
js/src/vm/Stack.cpp
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -684,35 +684,33 @@ EnclosingStaticScope(BytecodeEmitter *bc
 
     return bce->sc->asFunctionBox()->function();
 }
 
 #ifdef DEBUG
 static bool
 AllLocalsAliased(StaticBlockObject &obj)
 {
-    for (unsigned i = 0; i < obj.slotCount(); i++)
+    for (unsigned i = 0; i < obj.numVariables(); i++)
         if (!obj.isAliased(i))
             return false;
     return true;
 }
 #endif
 
 static bool
 ComputeAliasedSlots(ExclusiveContext *cx, BytecodeEmitter *bce, Handle<StaticBlockObject *> blockObj)
 {
-    if (blockObj->slotCount() == 0)
-        return true;
-
-    for (unsigned i = 0; i < blockObj->slotCount(); i++) {
+    for (unsigned i = 0; i < blockObj->numVariables(); i++) {
         Definition *dn = blockObj->definitionParseNode(i);
 
         JS_ASSERT(dn->isDefn());
         if (!dn->pn_cookie.set(bce->parser->tokenStream, dn->pn_cookie.level(),
-                               blockObj->varToLocalIndex(dn->frameSlot()))) {
+                               blockObj->blockIndexToLocalIndex(dn->frameSlot())))
+        {
             return false;
         }
 
 #ifdef DEBUG
         for (ParseNode *pnu = dn->dn_uses; pnu; pnu = pnu->pn_link) {
             JS_ASSERT(pnu->pn_lexdef == dn);
             JS_ASSERT(!(pnu->pn_dflags & PND_BOUND));
             JS_ASSERT(pnu->pn_cookie.isFree());
@@ -740,23 +738,23 @@ ComputeLocalOffset(ExclusiveContext *cx,
     unsigned nfixedvars = bce->sc->isFunctionBox() ? bce->script->bindings.numVars() : 0;
     unsigned localOffset = nfixedvars;
 
     if (bce->staticScope) {
         Rooted<NestedScopeObject *> outer(cx, bce->staticScope);
         for (; outer; outer = outer->enclosingNestedScope()) {
             if (outer->is<StaticBlockObject>()) {
                 StaticBlockObject &outerBlock = outer->as<StaticBlockObject>();
-                localOffset = outerBlock.localOffset() + outerBlock.slotCount();
+                localOffset = outerBlock.localOffset() + outerBlock.numVariables();
                 break;
             }
         }
     }
 
-    JS_ASSERT(localOffset + blockObj->slotCount()
+    JS_ASSERT(localOffset + blockObj->numVariables()
               <= nfixedvars + bce->script->bindings.numBlockScoped());
 
     blockObj->setLocalOffset(localOffset);
 }
 
 // ~ Nested Scopes ~
 //
 // A nested scope is a region of a compilation unit (function, script, or eval
@@ -2433,40 +2431,41 @@ PushUndefinedValues(ExclusiveContext *cx
     }
     return true;
 }
 
 static bool
 InitializeBlockScopedLocalsFromStack(ExclusiveContext *cx, BytecodeEmitter *bce,
                                      Handle<StaticBlockObject *> blockObj)
 {
-    for (unsigned i = blockObj->slotCount(); i > 0; --i) {
+    for (unsigned i = blockObj->numVariables(); i > 0; --i) {
         if (blockObj->isAliased(i - 1)) {
             ScopeCoordinate sc;
             sc.setHops(0);
             sc.setSlot(BlockObject::RESERVED_SLOTS + i - 1);
             if (!EmitAliasedVarOp(cx, JSOP_SETALIASEDVAR, sc, bce))
                 return false;
         } else {
-            if (!EmitUnaliasedVarOp(cx, JSOP_SETLOCAL, blockObj->varToLocalIndex(i - 1), bce))
+            unsigned local = blockObj->blockIndexToLocalIndex(i - 1);
+            if (!EmitUnaliasedVarOp(cx, JSOP_SETLOCAL, local, bce))
                 return false;
         }
         if (Emit1(cx, bce, JSOP_POP) < 0)
             return false;
     }
     return true;
 }
 
 static bool
 EnterBlockScope(ExclusiveContext *cx, BytecodeEmitter *bce, StmtInfoBCE *stmtInfo,
                 ObjectBox *objbox, unsigned alreadyPushed = 0)
 {
     // Initial values for block-scoped locals.
     Rooted<StaticBlockObject *> blockObj(cx, &objbox->object->as<StaticBlockObject>());
-    if (!PushUndefinedValues(cx, bce, blockObj->slotCount() - alreadyPushed))
+    if (!PushUndefinedValues(cx, bce, blockObj->numVariables() - alreadyPushed))
         return false;
 
     if (!EnterNestedScope(cx, bce, stmtInfo, objbox, STMT_BLOCK))
         return false;
 
     if (!InitializeBlockScopedLocalsFromStack(cx, bce, blockObj))
         return false;
 
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -2684,17 +2684,17 @@ Parser<FullParseHandler>::bindLet(BindDa
     ParseContext<FullParseHandler> *pc = parser->pc;
     ParseNode *pn = data->pn;
     if (!parser->checkStrictBinding(name, pn))
         return false;
 
     ExclusiveContext *cx = parser->context;
 
     Rooted<StaticBlockObject *> blockObj(cx, data->let.blockObj);
-    unsigned index = blockObj->slotCount();
+    unsigned index = blockObj->numVariables();
     if (index >= StaticBlockObject::LOCAL_INDEX_LIMIT) {
         parser->report(ParseError, false, pn, data->let.overflow);
         return false;
     }
 
     /*
      * Assign block-local index to pn->pn_cookie right away, encoding it as an
      * upvar cookie whose skip tells the current static level. The emitter will
@@ -2782,17 +2782,17 @@ struct PopLetDecl {
 template <typename ParseHandler>
 static void
 AccumulateBlockScopeDepth(ParseContext<ParseHandler> *pc)
 {
     uint32_t innerDepth = pc->topStmt->innerBlockScopeDepth;
     StmtInfoPC *outer = pc->topStmt->down;
 
     if (pc->topStmt->isBlockScope)
-        innerDepth += pc->topStmt->staticScope->template as<StaticBlockObject>().slotCount();
+        innerDepth += pc->topStmt->staticScope->template as<StaticBlockObject>().numVariables();
 
     if (outer) {
         if (outer->innerBlockScopeDepth < innerDepth)
             outer->innerBlockScopeDepth = innerDepth;
     } else {
         if (pc->blockScopeDepth < innerDepth)
             pc->blockScopeDepth = innerDepth;
     }
--- a/js/src/jit/BaselineFrame.cpp
+++ b/js/src/jit/BaselineFrame.cpp
@@ -65,17 +65,17 @@ BaselineFrame::trace(JSTracer *trc, IonF
 
         frameIterator.baselineScriptAndPc(nullptr, &pc);
         staticScope = script->getStaticScope(pc);
         while (staticScope && !staticScope->is<StaticBlockObject>())
             staticScope = staticScope->enclosingNestedScope();
 
         if (staticScope) {
             StaticBlockObject &blockObj = staticScope->as<StaticBlockObject>();
-            nlivefixed = blockObj.localOffset() + blockObj.slotCount();
+            nlivefixed = blockObj.localOffset() + blockObj.numVariables();
         }
     }
 
     JS_ASSERT(nlivefixed <= nfixed);
     JS_ASSERT(nlivefixed >= script->nfixedvars());
 
     if (nfixed == nlivefixed) {
         // All locals are live.
--- a/js/src/jit/CompileInfo.h
+++ b/js/src/jit/CompileInfo.h
@@ -254,17 +254,17 @@ class CompileInfo
                 return script()->varIsAliased(local);
 
             // Otherwise, it might be part of a block scope.
             for (; staticScope; staticScope = staticScope->enclosingNestedScope()) {
                 if (!staticScope->is<StaticBlockObject>())
                     continue;
                 StaticBlockObject &blockObj = staticScope->as<StaticBlockObject>();
                 if (blockObj.localOffset() < local) {
-                    if (local - blockObj.localOffset() < blockObj.slotCount())
+                    if (local - blockObj.localOffset() < blockObj.numVariables())
                         return blockObj.isAliased(local - blockObj.localOffset());
                     return false;
                 }
             }
 
             // In this static scope, this var is dead.
             return false;
         }
--- a/js/src/jsopcode.cpp
+++ b/js/src/jsopcode.cpp
@@ -1441,17 +1441,17 @@ struct ExpressionDecompiler
           localNames(nullptr),
           parser(cx, script),
           sprinter(cx)
     {}
     ~ExpressionDecompiler();
     bool init();
     bool decompilePCForStackOperand(jsbytecode *pc, int i);
     bool decompilePC(jsbytecode *pc);
-    JSAtom *getFixed(uint32_t slot, jsbytecode *pc);
+    JSAtom *getLocal(uint32_t local, jsbytecode *pc);
     JSAtom *getArg(unsigned slot);
     JSAtom *loadAtom(jsbytecode *pc);
     bool quote(JSString *s, uint32_t quote);
     bool write(const char *s);
     bool write(JSString *str);
     bool getOutput(char **out);
 };
 
@@ -1508,17 +1508,17 @@ ExpressionDecompiler::decompilePC(jsbyte
       case JSOP_CALLARG: {
         unsigned slot = GET_ARGNO(pc);
         JSAtom *atom = getArg(slot);
         return write(atom);
       }
       case JSOP_GETLOCAL:
       case JSOP_CALLLOCAL: {
         uint32_t i = GET_LOCALNO(pc);
-        if (JSAtom *atom = getFixed(i, pc))
+        if (JSAtom *atom = getLocal(i, pc))
             return write(atom);
         return write("(intermediate value)");
       }
       case JSOP_CALLALIASEDVAR:
       case JSOP_GETALIASEDVAR: {
         JSAtom *atom = ScopeCoordinateName(cx->runtime()->scopeCoordinateNameCache, script, pc);
         JS_ASSERT(atom);
         return write(atom);
@@ -1644,38 +1644,39 @@ JSAtom *
 ExpressionDecompiler::getArg(unsigned slot)
 {
     JS_ASSERT(fun);
     JS_ASSERT(slot < script->bindings.count());
     return (*localNames)[slot].name();
 }
 
 JSAtom *
-ExpressionDecompiler::getFixed(uint32_t slot, jsbytecode *pc)
+ExpressionDecompiler::getLocal(uint32_t local, jsbytecode *pc)
 {
-    if (slot < script->nfixedvars()) {
+    JS_ASSERT(local < script->nfixed());
+    if (local < script->nfixedvars()) {
         JS_ASSERT(fun);
-        slot += fun->nargs();
+        uint32_t slot = local + fun->nargs();
         JS_ASSERT(slot < script->bindings.count());
         return (*localNames)[slot].name();
     }
     for (NestedScopeObject *chain = script->getStaticScope(pc);
          chain;
          chain = chain->enclosingNestedScope()) {
         if (!chain->is<StaticBlockObject>())
             continue;
         StaticBlockObject &block = chain->as<StaticBlockObject>();
-        if (slot < block.localOffset())
+        if (local < block.localOffset())
             continue;
-        slot -= block.localOffset();
-        if (slot >= block.slotCount())
+        local -= block.localOffset();
+        if (local >= block.numVariables())
             return nullptr;
         for (Shape::Range<NoGC> r(block.lastProperty()); !r.empty(); r.popFront()) {
             const Shape &shape = r.front();
-            if (block.shapeToIndex(shape) == slot)
+            if (block.shapeToIndex(shape) == local)
                 return JSID_TO_ATOM(shape.propid());
         }
         break;
     }
     return nullptr;
 }
 
 bool
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -658,42 +658,46 @@ ClonedBlockObject::create(JSContext *cx,
     if (&frame.scopeChain()->global() != obj->getParent()) {
         JS_ASSERT(obj->getParent() == nullptr);
         Rooted<GlobalObject*> global(cx, &frame.scopeChain()->global());
         if (!JSObject::setParent(cx, obj, global))
             return nullptr;
     }
 
     JS_ASSERT(!obj->inDictionaryMode());
-    JS_ASSERT(obj->slotSpan() >= block->slotCount() + RESERVED_SLOTS);
+    JS_ASSERT(obj->slotSpan() >= block->numVariables() + RESERVED_SLOTS);
 
     obj->setReservedSlot(SCOPE_CHAIN_SLOT, ObjectValue(*frame.scopeChain()));
 
     /*
      * Copy in the closed-over locals. Closed-over locals don't need
      * any fixup since the initial value is 'undefined'.
      */
-    unsigned nslots = block->slotCount();
-    for (unsigned i = 0; i < nslots; ++i) {
-        if (block->isAliased(i))
-            obj->as<ClonedBlockObject>().setVar(i, frame.unaliasedLocal(block->varToLocalIndex(i)));
+    unsigned nvars = block->numVariables();
+    for (unsigned i = 0; i < nvars; ++i) {
+        if (block->isAliased(i)) {
+            Value &val = frame.unaliasedLocal(block->blockIndexToLocalIndex(i));
+            obj->as<ClonedBlockObject>().setVar(i, val);
+        }
     }
 
     JS_ASSERT(obj->isDelegate());
 
     return &obj->as<ClonedBlockObject>();
 }
 
 void
 ClonedBlockObject::copyUnaliasedValues(AbstractFramePtr frame)
 {
     StaticBlockObject &block = staticBlock();
-    for (unsigned i = 0; i < slotCount(); ++i) {
-        if (!block.isAliased(i))
-            setVar(i, frame.unaliasedLocal(block.varToLocalIndex(i)), DONT_CHECK_ALIASING);
+    for (unsigned i = 0; i < numVariables(); ++i) {
+        if (!block.isAliased(i)) {
+            Value &val = frame.unaliasedLocal(block.blockIndexToLocalIndex(i));
+            setVar(i, val, DONT_CHECK_ALIASING);
+        }
     }
 }
 
 StaticBlockObject *
 StaticBlockObject::create(ExclusiveContext *cx)
 {
     RootedTypeObject type(cx, cx->getNewType(&BlockObject::class_, nullptr));
     if (!type)
@@ -766,17 +770,17 @@ js::XDRStaticBlockObject(XDRState<mode> 
 
     JSContext *cx = xdr->cx();
 
     Rooted<StaticBlockObject*> obj(cx);
     uint32_t count = 0, offset = 0;
 
     if (mode == XDR_ENCODE) {
         obj = *objp;
-        count = obj->slotCount();
+        count = obj->numVariables();
         offset = obj->localOffset();
     }
 
     if (mode == XDR_DECODE) {
         obj = StaticBlockObject::create(cx);
         if (!obj)
             return false;
         obj->initEnclosingNestedScope(enclosingScope);
@@ -871,17 +875,17 @@ CloneStaticBlockObject(JSContext *cx, Ha
     if (!clone)
         return nullptr;
 
     clone->initEnclosingNestedScope(enclosingScope);
     clone->setLocalOffset(srcBlock->localOffset());
 
     /* Shape::Range is reverse order, so build a list in forward order. */
     AutoShapeVector shapes(cx);
-    if (!shapes.growBy(srcBlock->slotCount()))
+    if (!shapes.growBy(srcBlock->numVariables()))
         return nullptr;
 
     for (Shape::Range<NoGC> r(srcBlock->lastProperty()); !r.empty(); r.popFront())
         shapes[srcBlock->shapeToIndex(r.front())] = &r.front();
 
     for (Shape **p = shapes.begin(); p != shapes.end(); ++p) {
         RootedId id(cx, (*p)->propid());
         unsigned i = srcBlock->shapeToIndex(**p);
@@ -1253,17 +1257,17 @@ class DebugScopeProxy : public BaseProxy
                 return false;
 
             unsigned i = block->staticBlock().shapeToIndex(*shape);
             if (block->staticBlock().isAliased(i))
                 return false;
 
             if (maybeLiveScope) {
                 AbstractFramePtr frame = maybeLiveScope->frame();
-                uint32_t local = block->staticBlock().varToLocalIndex(i);
+                uint32_t local = block->staticBlock().blockIndexToLocalIndex(i);
                 JS_ASSERT(local < frame.script()->nfixed());
                 if (action == GET)
                     vp.set(frame.unaliasedLocal(local));
                 else
                     frame.unaliasedLocal(local) = vp;
             } else {
                 if (action == GET)
                     vp.set(block->var(i, DONT_CHECK_ALIASING));
--- a/js/src/vm/ScopeObject.h
+++ b/js/src/vm/ScopeObject.h
@@ -401,17 +401,18 @@ class BlockObject : public NestedScopeOb
     static const Class class_;
 
     /* Return the abstract stack depth right before entering this nested scope. */
     uint32_t stackDepth() const {
         return getReservedSlot(DEPTH_SLOT).toPrivateUint32();
     }
 
     /* Return the number of variables associated with this block. */
-    uint32_t slotCount() const {
+    uint32_t numVariables() const {
+        // TODO: propertyCount() is O(n), use O(1) lastProperty()->slot() instead
         return propertyCount();
     }
 
   protected:
     /* Blocks contain an object slot for each slot i: 0 <= i < slotCount. */
     const Value &slotValue(unsigned i) {
         return getSlotRef(RESERVED_SLOTS + i);
     }
@@ -429,49 +430,49 @@ class StaticBlockObject : public BlockOb
     static StaticBlockObject *create(ExclusiveContext *cx);
 
     /* See StaticScopeIter comment. */
     JSObject *enclosingStaticScope() const {
         return getFixedSlot(SCOPE_CHAIN_SLOT).toObjectOrNull();
     }
 
     /*
-     * Return the index (in the range [0, slotCount()) corresponding to the
+     * Return the index (in the range [0, numVariables()) corresponding to the
      * given shape of a block object.
      */
     uint32_t shapeToIndex(const Shape &shape) {
         uint32_t slot = shape.slot();
-        JS_ASSERT(slot - RESERVED_SLOTS < slotCount());
+        JS_ASSERT(slot - RESERVED_SLOTS < numVariables());
         return slot - RESERVED_SLOTS;
     }
 
     /*
      * A refinement of enclosingStaticScope that returns nullptr if the enclosing
      * static scope is a JSFunction.
      */
     inline StaticBlockObject *enclosingBlock() const;
 
     uint32_t localOffset() {
         return getReservedSlot(LOCAL_OFFSET_SLOT).toPrivateUint32();
     }
 
     // Return the local corresponding to the 'var'th binding where 'var' is in the
-    // range [0, slotCount()).
-    uint32_t varToLocalIndex(uint32_t var) {
-        JS_ASSERT(var < slotCount());
-        return getReservedSlot(LOCAL_OFFSET_SLOT).toPrivateUint32() + var;
+    // range [0, numVariables()).
+    uint32_t blockIndexToLocalIndex(uint32_t index) {
+        JS_ASSERT(index < numVariables());
+        return getReservedSlot(LOCAL_OFFSET_SLOT).toPrivateUint32() + index;
     }
 
     // Return the slot corresponding to local variable 'local', where 'local' is
-    // in the range [localOffset(), localOffset() + slotCount()).  The result is
-    // in the range [RESERVED_SLOTS, RESERVED_SLOTS + slotCount()).
+    // in the range [localOffset(), localOffset() + numVariables()).  The result is
+    // in the range [RESERVED_SLOTS, RESERVED_SLOTS + numVariables()).
     uint32_t localIndexToSlot(uint32_t local) {
         JS_ASSERT(local >= localOffset());
         local -= localOffset();
-        JS_ASSERT(local < slotCount());
+        JS_ASSERT(local < numVariables());
         return RESERVED_SLOTS + local;
     }
 
     /*
      * A let binding is aliased if accessed lexically by nested functions or
      * dynamically through dynamic name lookup (eval, with, function::, etc).
      */
     bool isAliased(unsigned i) {
--- a/js/src/vm/Stack.cpp
+++ b/js/src/vm/Stack.cpp
@@ -393,17 +393,17 @@ StackFrame::markValues(JSTracer *trc, Va
     while (staticScope && !staticScope->is<StaticBlockObject>())
         staticScope = staticScope->enclosingNestedScope();
 
     size_t nfixed = script()->nfixed();
     size_t nlivefixed;
 
     if (staticScope) {
         StaticBlockObject &blockObj = staticScope->as<StaticBlockObject>();
-        nlivefixed = blockObj.localOffset() + blockObj.slotCount();
+        nlivefixed = blockObj.localOffset() + blockObj.numVariables();
     } else {
         nlivefixed = script()->nfixedvars();
     }
 
     if (nfixed == nlivefixed) {
         // All locals are live.
         MarkLocals(this, trc, 0, sp - slots());
     } else {