Bug 1066197 part 1 - Some js_IteratorMore cleanup. r=bhackett

js/src/jit/BaselineIC.cpp

@@ -9830,7 +9830,7 @@ ICIteratorNew_Fallback::Compiler::generateStubCode(MacroAssembler &masm)
 
 static bool
 DoIteratorMoreFallback(JSContext *cx, BaselineFrame *frame, ICIteratorMore_Fallback *stub_,
-                       HandleValue iterValue, MutableHandleValue res)
+                       HandleObject iterObj, MutableHandleValue res)
 {
     // This fallback stub may trigger debug mode toggling.
     DebugModeOSRVolatileStub<ICIteratorMore_Fallback *> stub(frame, stub_);
@@ -9838,7 +9838,7 @@ DoIteratorMoreFallback(JSContext *cx, BaselineFrame *frame, ICIteratorMore_Fallb
     FallbackICSpew(cx, stub, "IteratorMore");
 
     bool cond;
-    if (!IteratorMore(cx, &iterValue.toObject(), &cond, res))
+    if (!js_IteratorMore(cx, iterObj, &cond))
         return false;
     res.setBoolean(cond);
 
@@ -9846,7 +9846,7 @@ DoIteratorMoreFallback(JSContext *cx, BaselineFrame *frame, ICIteratorMore_Fallb
     if (stub.invalid())
         return true;
 
-    if (iterValue.toObject().is<PropertyIteratorObject>() &&
+    if (iterObj->is<PropertyIteratorObject>() &&
         !stub->hasStub(ICStub::IteratorMore_Native))
     {
         ICIteratorMore_Native::Compiler compiler(cx);
@@ -9860,7 +9860,7 @@ DoIteratorMoreFallback(JSContext *cx, BaselineFrame *frame, ICIteratorMore_Fallb
 }
 
 typedef bool (*DoIteratorMoreFallbackFn)(JSContext *, BaselineFrame *, ICIteratorMore_Fallback *,
-                                         HandleValue, MutableHandleValue);
+                                         HandleObject, MutableHandleValue);
 static const VMFunction DoIteratorMoreFallbackInfo =
     FunctionInfo<DoIteratorMoreFallbackFn>(DoIteratorMoreFallback);
 
@@ -9869,7 +9869,8 @@ ICIteratorMore_Fallback::Compiler::generateStubCode(MacroAssembler &masm)
 {
     EmitRestoreTailCallReg(masm);
 
-    masm.pushValue(R0);
+    masm.unboxObject(R0, R0.scratchReg());
+    masm.push(R0.scratchReg());
     masm.push(BaselineStubReg);
     masm.pushBaselineFramePtr(BaselineFrameReg, R0.scratchReg());
 

js/src/jit/CodeGenerator.cpp

@@ -6400,7 +6400,7 @@ CodeGenerator::visitIteratorNext(LIteratorNext *lir)
 }
 
 typedef bool (*IteratorMoreFn)(JSContext *, HandleObject, bool *);
-static const VMFunction IteratorMoreInfo = FunctionInfo<IteratorMoreFn>(jit::IteratorMore);
+static const VMFunction IteratorMoreInfo = FunctionInfo<IteratorMoreFn>(js_IteratorMore);
 
 bool
 CodeGenerator::visitIteratorMore(LIteratorMore *lir)

js/src/jit/VMFunctions.cpp

@@ -297,17 +297,6 @@ StringsEqual(JSContext *cx, HandleString lhs, HandleString rhs, bool *res)
 template bool StringsEqual<true>(JSContext *cx, HandleString lhs, HandleString rhs, bool *res);
 template bool StringsEqual<false>(JSContext *cx, HandleString lhs, HandleString rhs, bool *res);
 
-bool
-IteratorMore(JSContext *cx, HandleObject obj, bool *res)
-{
-    RootedValue tmp(cx);
-    if (!js_IteratorMore(cx, obj, &tmp))
-        return false;
-
-    *res = tmp.toBoolean();
-    return true;
-}
-
 JSObject*
 NewInitArray(JSContext *cx, uint32_t count, types::TypeObject *typeArg)
 {

js/src/jit/VMFunctions.h

@@ -631,8 +631,6 @@ bool GreaterThanOrEqual(JSContext *cx, MutableHandleValue lhs, MutableHandleValu
 template<bool Equal>
 bool StringsEqual(JSContext *cx, HandleString left, HandleString right, bool *res);
 
-bool IteratorMore(JSContext *cx, HandleObject obj, bool *res);
-
 // Allocation functions for JSOP_NEWARRAY and JSOP_NEWOBJECT and parallel array inlining
 JSObject *NewInitParallelArray(JSContext *cx, HandleObject templateObj);
 JSObject *NewInitArray(JSContext *cx, uint32_t count, types::TypeObject *type);

js/src/jsiter.cpp

@@ -864,10 +864,11 @@ iterator_next_impl(JSContext *cx, CallArgs args)
 
     RootedObject thisObj(cx, &args.thisv().toObject());
 
-    if (!js_IteratorMore(cx, thisObj, args.rval()))
+    bool more;
+    if (!js_IteratorMore(cx, thisObj, &more))
         return false;
 
-    if (!args.rval().toBoolean()) {
+    if (!more) {
         js_ThrowStopIteration(cx);
         return false;
     }
@@ -1253,7 +1254,7 @@ js_SuppressDeletedElements(JSContext *cx, HandleObject obj, uint32_t begin, uint
 }
 
 bool
-js_IteratorMore(JSContext *cx, HandleObject iterobj, MutableHandleValue rval)
+js_IteratorMore(JSContext *cx, HandleObject iterobj, bool *res)
 {
     /* Fast path for native iterators */
     NativeIterator *ni = nullptr;
@@ -1262,14 +1263,14 @@ js_IteratorMore(JSContext *cx, HandleObject iterobj, MutableHandleValue rval)
         ni = iterobj->as<PropertyIteratorObject>().getNativeIterator();
         bool more = ni->props_cursor < ni->props_end;
         if (ni->isKeyIter() || !more) {
-            rval.setBoolean(more);
+            *res = more;
             return true;
         }
     }
 
     /* We might still have a pending value. */
     if (!cx->iterValue.isMagic(JS_NO_ITER_VALUE)) {
-        rval.setBoolean(true);
+        *res = true;
         return true;
     }
 
@@ -1277,6 +1278,7 @@ js_IteratorMore(JSContext *cx, HandleObject iterobj, MutableHandleValue rval)
     JS_CHECK_RECURSION(cx, return false);
 
     /* Fetch and cache the next value from the iterator. */
+    RootedValue val(cx);
     if (ni) {
         JS_ASSERT(!ni->isKeyIter());
         RootedId id(cx);
@@ -1285,15 +1287,15 @@ js_IteratorMore(JSContext *cx, HandleObject iterobj, MutableHandleValue rval)
             return false;
         ni->incCursor();
         RootedObject obj(cx, ni->obj);
-        if (!JSObject::getGeneric(cx, obj, obj, id, rval))
+        if (!JSObject::getGeneric(cx, obj, obj, id, &val))
             return false;
-        if ((ni->flags & JSITER_KEYVALUE) && !NewKeyValuePair(cx, id, rval, rval))
+        if ((ni->flags & JSITER_KEYVALUE) && !NewKeyValuePair(cx, id, val, &val))
             return false;
     } else {
         /* Call the iterator object's .next method. */
-        if (!JSObject::getProperty(cx, iterobj, iterobj, cx->names().next, rval))
+        if (!JSObject::getProperty(cx, iterobj, iterobj, cx->names().next, &val))
             return false;
-        if (!Invoke(cx, ObjectValue(*iterobj), rval, 0, nullptr, rval)) {
+        if (!Invoke(cx, ObjectValue(*iterobj), val, 0, nullptr, &val)) {
             /* Check for StopIteration. */
             if (!cx->isExceptionPending())
                 return false;
@@ -1305,15 +1307,15 @@ js_IteratorMore(JSContext *cx, HandleObject iterobj, MutableHandleValue rval)
 
             cx->clearPendingException();
             cx->iterValue.setMagic(JS_NO_ITER_VALUE);
-            rval.setBoolean(false);
+            *res = false;
             return true;
         }
     }
 
     /* Cache the value returned by iterobj.next() so js_IteratorNext() can find it. */
-    JS_ASSERT(!rval.isMagic(JS_NO_ITER_VALUE));
-    cx->iterValue = rval;
-    rval.setBoolean(true);
+    JS_ASSERT(!val.isMagic(JS_NO_ITER_VALUE));
+    cx->iterValue = val;
+    *res = true;
     return true;
 }
 

js/src/jsiter.h

@@ -208,7 +208,7 @@ js_SuppressDeletedElements(JSContext *cx, js::HandleObject obj, uint32_t begin,
  * picked up by IteratorNext(). The value is cached in the current context.
  */
 extern bool
-js_IteratorMore(JSContext *cx, js::HandleObject iterobj, js::MutableHandleValue rval);
+js_IteratorMore(JSContext *cx, js::HandleObject iterobj, bool *res);
 
 extern bool
 js_IteratorNext(JSContext *cx, js::HandleObject iterobj, js::MutableHandleValue rval);

js/src/vm/Interpreter.cpp

@@ -1116,27 +1116,6 @@ JS_STATIC_ASSERT(JSOP_SETNAME_LENGTH == JSOP_SETPROP_LENGTH);
 JS_STATIC_ASSERT(JSOP_IFNE_LENGTH == JSOP_IFEQ_LENGTH);
 JS_STATIC_ASSERT(JSOP_IFNE == JSOP_IFEQ + 1);
 
-/*
- * Inline fast paths for iteration. js_IteratorMore and js_IteratorNext handle
- * all cases, but we inline the most frequently taken paths here.
- */
-bool
-js::IteratorMore(JSContext *cx, JSObject *iterobj, bool *cond, MutableHandleValue rval)
-{
-    if (iterobj->is<PropertyIteratorObject>()) {
-        NativeIterator *ni = iterobj->as<PropertyIteratorObject>().getNativeIterator();
-        if (ni->isKeyIter()) {
-            *cond = (ni->props_cursor < ni->props_end);
-            return true;
-        }
-    }
-    Rooted<JSObject*> iobj(cx, iterobj);
-    if (!js_IteratorMore(cx, iobj, rval))
-        return false;
-    *cond = rval.isTrue();
-    return true;
-}
-
 bool
 js::IteratorNext(JSContext *cx, HandleObject iterobj, MutableHandleValue rval)
 {
@@ -1927,9 +1906,10 @@ CASE(JSOP_MOREITER)
     JS_ASSERT(REGS.stackDepth() >= 1);
     JS_ASSERT(REGS.sp[-1].isObject());
     PUSH_NULL();
+    RootedObject &obj = rootObject0;
+    obj = &REGS.sp[-2].toObject();
     bool cond;
-    MutableHandleValue res = REGS.stackHandleAt(-1);
-    if (!IteratorMore(cx, &REGS.sp[-2].toObject(), &cond, res))
+    if (!js_IteratorMore(cx, obj, &cond))
         goto error;
     REGS.sp[-1].setBoolean(cond);
 }

js/src/vm/Interpreter.h

@@ -439,9 +439,6 @@ bool
 ImplicitThisOperation(JSContext *cx, HandleObject scopeObj, HandlePropertyName name,
                       MutableHandleValue res);
 
-bool
-IteratorMore(JSContext *cx, JSObject *iterobj, bool *cond, MutableHandleValue rval);
-
 bool
 IteratorNext(JSContext *cx, HandleObject iterobj, MutableHandleValue rval);