Just noticed this when testing my jsop_case patch. jit-test/tests/basic/testBug634590.js:12: Error: Assertion failed: got "outerouterouterouterouterouterouterouterouterouter", expected "innerinnerinnerinnerinnerinnerinnerinnerinnerinner" Bug 634590 is/was a hardblocker, so asking blocking-2.0.

Need help from the JM guys.

Andreas, the patch in bug 634590 changed jsinterp but not StubCalls.cpp?

I thought we did, but maybe not right.

This fixes the bug. It causes no perf regression on V8, but there is a 3 ms on SunSpider with -m -a and -m -j -p. I'll try to add a fast path for the common case.

This one seems not to regress perf. I'll post a cleaned-up version for review in a bit.

Comment on attachment 515254 [details] [diff] [review] Patch Logic seems sound. mjit details dvander has to review.

Brian noticed this and mentioned it on IRC today -- sorry I didn't report it then. /be

Comment on attachment 515254 [details] [diff] [review] Patch >+ /* If the callee is not an object, jump to the inline fast path. */ >+ MaybeJump isNotObj; >+ if (!fval->isType(JSVAL_TYPE_OBJECT)) >+ isNotObj = frame.testObject(Assembler::NotEqual, fval); >+ >+ /* >+ * If the callee is not a function, jump to OOL slow path. >+ */ >+ RegisterID objReg = frame.tempRegForData(fval); If the earlier branch was emitted, then this second tempRegForData call could evict something, causing a register state mismatch at the join point. What we probably want to do is: > MaybeRegisterID typeReg = frame.maybePinType(fval); > RegisterID objReg = frame.copyDataIntoReg(fval); > > MaybeJump isNotObj; > if (!fval->isType(JSVAL_TYPE_OBJECT)) { > isNotObj = frame.testObject(Assembler::NotEqual, fval); > frame.maybeUnpinType(typeReg); > } > >+ RegisterID parentReg = frame.allocReg(); Same here, this could evict inside a control-flow arm, but with the copyRegToData() call above, we can just clobber objReg.

Is this ready to land?

Yes.

Backed out due to Unix build failures: http://hg.mozilla.org/tracemonkey/rev/a7c87d6ee78f Should be able to sort it out this afternoon.

Relanded: http://hg.mozilla.org/tracemonkey/rev/83242d9362cd

The added test testBug634590ma.js fails for me when run with '-m -a': /home/njn/moz/ws7/js/src/debug32/shell/js -m -a -e "const platform='linux2'; const libdir='/home/njn/moz/ws7/js/src/jit-test/lib/';" -f /home/njn/moz/ws7/js/src/jit-test/lib/prolog.js -f /home/njn/moz/ws7/js/src/jit-test/tests/basic/testBug634590ma.js /home/njn/moz/ws7/js/src/jit-test/tests/basic/testBug634590ma.js:14: Error: Assertion failed: got "outerouterouterouterouterouterouterouterouterouter", expected "innerinnerinnerinnerinnerinnerinnerinnerinnerinner" This is preventing me from landing bug 635155.

(In reply to comment #16) > The added test testBug634590ma.js fails for me when run with '-m -a': It's working now. I think I forgot to rebuild after updating. Sorry for the noise.

(In reply to comment #15) > Relanded: > > http://hg.mozilla.org/tracemonkey/rev/83242d9362cd Can we get this on m-c for tonight's nightly?

(In reply to comment #18) > (In reply to comment #15) > > Relanded: > > > > http://hg.mozilla.org/tracemonkey/rev/83242d9362cd > > Can we get this on m-c for tonight's nightly? http://hg.mozilla.org/mozilla-central/rev/ce9aafdae381