[INFER] Reload from adjusted return address when rejoining from lowered call or apply, bug 651119.
authorBrian Hackett <bhackett1024@gmail.com>
Tue, 19 Apr 2011 21:33:04 -0700
changeset 74961 44392a434cb1eda40ea0d17330a1ed2ea01da346
parent 74960 fbcbc74151c16215a3a24d9377684e7b40a7e31a
child 74962 3538d4d61e0ec1de3c4228073f7aaf39f647881d
push id2
push userbsmedberg@mozilla.com
push dateFri, 19 Aug 2011 14:38:13 +0000
bugs651119
milestone6.0a1
[INFER] Reload from adjusted return address when rejoining from lowered call or apply, bug 651119.
js/src/jit-test/tests/jaeger/recompile/bug651119.js
js/src/methodjit/Compiler.cpp
js/src/methodjit/FrameState-inl.h
js/src/methodjit/FrameState.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/jaeger/recompile/bug651119.js
@@ -0,0 +1,51 @@
+Object.extend = function(destination, source) {
+    for (var property in source)
+        destination[property] = source[property];
+};
+var Enumerable = {
+    _each: function(iterator) {
+        for (var i = 0, length = this.length; i < length; i++)
+            iterator(this[i]);
+    },
+    each: function(iterator, context) {
+        var index = 0;
+        this._each(function(value) {
+            iterator.call(context, value, index++);
+        });
+    },
+    map: function(iterator, context) {
+        var results = [];
+        this.each(function(value, index) {
+            var res = iterator.call(context, value);
+            results.push(res);
+        });
+        return results;
+    },
+    invoke: function(method) {
+        var args = $A(arguments).slice(1);
+        return this.map(function(value) {
+            return value[method].apply(value, args);
+        });
+    },
+};
+Object.extend(Array.prototype, Enumerable);
+function $A(iterable) {
+    var length = iterable.length || 0, results = new Array(length);
+    while (length--) results[length] = iterable[length];
+    return results;
+}
+function g() {
+    return [1, 2, 3, 4, 5].each(function(part) {
+        return 0;
+    });
+}
+function f() {
+    g();
+    g();
+    g();
+    g();
+    var result = [[2, 1, 3], [6, 5, 4]];    
+    result = result.invoke('invoke', 'toString', 2);
+    result[0].join(', ');
+};
+f();
--- a/js/src/methodjit/Compiler.cpp
+++ b/js/src/methodjit/Compiler.cpp
@@ -2060,22 +2060,45 @@ mjit::Compiler::generateMethod()
                 stubcc.masm.jump(Registers::ReturnReg);
 
                 callPatch.joinPoint = stubcc.masm.label();
                 callPatch.joinSlow = true;
 
                 addReturnSite(true /* ool */);
 
                 autoRejoinNcode.oolRejoin(stubcc.masm.label());
-                stubcc.masm.storeValueFromComponents(JSReturnReg_Type, JSReturnReg_Data,
-                                                     frame.addressOf(frame.peek(-1)));
+
+                /*
+                 * Watch for lowered calls and applies, which behave differently
+                 * from every other rejoin path in JM and require the return
+                 * value to be loaded from a different address.
+                 */
+                bool loweredCallApply =
+                    (frameSize.isDynamic() || frameSize.staticArgc() != GET_ARGC(PC));
+                Address address = frame.addressOf(frame.peek(-1));
+                if (loweredCallApply) {
+                    frame.pushSynced(JSVAL_TYPE_UNKNOWN);
+                    address = frame.addressOf(frame.peek(-1));
+                    frame.pop();
+                }
+
+                stubcc.masm.storeValueFromComponents(JSReturnReg_Type, JSReturnReg_Data, address);
                 fallthrough.linkTo(stubcc.masm.label(), &stubcc.masm);
-                if (knownPushedType(0) == JSVAL_TYPE_DOUBLE)
-                    stubcc.masm.ensureInMemoryDouble(frame.addressOf(frame.peek(-1)));
-                stubcc.rejoin(Changes(1));
+
+                /*
+                 * We can't just reload from the return address as inlineCallHelper
+                 * does, as if we inlined a call here there may be other registers
+                 * that need to be reloaded.
+                 */
+                if (loweredCallApply) {
+                    frame.reloadEntry(stubcc.masm, address, frame.peek(-1));
+                    stubcc.crossJump(stubcc.masm.jump(), masm.label());
+                } else {
+                    stubcc.rejoin(Changes(1));
+                }
 
                 callPatches.append(callPatch);
             }
           } }
           END_CASE(JSOP_CALL)
 
           BEGIN_CASE(JSOP_NAME)
           {
@@ -3717,32 +3740,29 @@ mjit::Compiler::inlineCallHelper(uint32 
      */
     CHECK_IC_SPACE();
 
     JSValueType type = knownPushedType(0);
 
     frame.popn(speculatedArgc + 2);
     frame.takeReg(JSReturnReg_Type);
     frame.takeReg(JSReturnReg_Data);
-    FPRegisterID fpreg = frame.pushRegs(JSReturnReg_Type, JSReturnReg_Data, type);
+    frame.pushRegs(JSReturnReg_Type, JSReturnReg_Data, type);
 
     /*
      * Now that the frame state is set, generate the rejoin path. Note that, if
      * lowerFunCallOrApply, we cannot just call 'stubcc.rejoin' since the return
      * value has been placed at vp[1] which is not the stack address associated
      * with frame.peek(-1).
      */
     callIC.slowJoinPoint = stubcc.masm.label();
     rejoin1.linkTo(callIC.slowJoinPoint, &stubcc.masm);
     rejoin2.linkTo(callIC.slowJoinPoint, &stubcc.masm);
     JaegerSpew(JSpew_Insns, " ---- BEGIN SLOW RESTORE CODE ---- \n");
-    if (type == JSVAL_TYPE_DOUBLE)
-        stubcc.masm.moveInt32OrDouble(icRvalAddr, fpreg);
-    else
-        stubcc.masm.loadValueAsComponents(icRvalAddr, JSReturnReg_Type, JSReturnReg_Data);
+    frame.reloadEntry(stubcc.masm, icRvalAddr, frame.peek(-1));
     stubcc.crossJump(stubcc.masm.jump(), masm.label());
     JaegerSpew(JSpew_Insns, " ---- END SLOW RESTORE CODE ---- \n");
 
     CHECK_OOL_SPACE();
 
     if (lowerFunCallOrApply)
         stubcc.crossJump(uncachedCallSlowRejoin, masm.label());
 
--- a/js/src/methodjit/FrameState-inl.h
+++ b/js/src/methodjit/FrameState-inl.h
@@ -371,16 +371,32 @@ FrameState::pushRegs(RegisterID type, Re
     }
 
     freeReg(type);
     pushTypedPayload(knownType, data);
     return Registers::FPConversionTemp;
 }
 
 inline void
+FrameState::reloadEntry(Assembler &masm, Address address, FrameEntry *fe)
+{
+    if (fe->data.inRegister()) {
+        if (fe->type.inRegister()) {
+            masm.loadValueAsComponents(address, fe->type.reg(), fe->data.reg());
+        } else {
+            JS_ASSERT(fe->isTypeKnown());
+            masm.loadPayload(address, fe->data.reg());
+        }
+    } else {
+        JS_ASSERT(fe->data.inFPRegister());
+        masm.moveInt32OrDouble(address, fe->data.fpreg());
+    }
+}
+
+inline void
 FrameState::pushTypedPayload(JSValueType type, RegisterID payload)
 {
     JS_ASSERT(type != JSVAL_TYPE_DOUBLE);
     JS_ASSERT(!a->freeRegs.hasReg(payload));
 
     FrameEntry *fe = rawPush();
 
     fe->resetUnsynced();
--- a/js/src/methodjit/FrameState.h
+++ b/js/src/methodjit/FrameState.h
@@ -275,16 +275,22 @@ class FrameState
 
     /*
      * Pushes a type register and data register pair, converting to the specified
      * known type if necessary.  If the type is JSVAL_TYPE_DOUBLE, the registers
      * are converted into a floating point register, which is returned.
      */
     inline FPRegisterID pushRegs(RegisterID type, RegisterID data, JSValueType knownType);
 
+    /*
+     * Load an address into a frame entry in registers. For handling call paths
+     * where merge() would otherwise reload from the wrong address.
+     */
+    inline void reloadEntry(Assembler &masm, Address address, FrameEntry *fe);
+
     /* Push a value which is definitely a double. */
     void pushDouble(FPRegisterID fpreg);
     void pushDouble(Address address);
 
     /* Ensure that fe is definitely a double.  It must already be either int or double. */
     void ensureDouble(FrameEntry *fe);
 
     /*