…/js --ion -n …/jit-test/tests/ion/bug691747.js Assertion failure: NYI, at /home/nicolas/mozilla/ionmonkey/js/src/ion/Lowering.cpp:381

No review asked, because the patch still need some clean-up and it need to be tested on ARM.

Fix: - Argument order of the interpreter. - *AtStart LUse. - (AdjustInput) Unbox only if boxed.

Currently the way of getting the script & pc from the frame and the snapshot is wrong because it could be an inlined frame. We need the script and the pc to monitor types.

Comment on attachment 586330 [details] [diff] [review] Implement MConcat and MAddGeneric. Review of attachment 586330 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/IonBuilder.cpp @@ +1949,5 @@ > + return true; > +} > + > +bool > +IonBuilder::jsop_add_fastPath(MDefinition *left, MDefinition *right) s/fastPath/specialize/ @@ +1957,5 @@ > + if (!b.lhsTypes || !b.rhsTypes || !b.outTypes) > + return false; > + > + if (b.outTypes->getKnownTypeTag(cx) == JSVAL_TYPE_INT32 || > + b.outTypes->getKnownTypeTag(cx) == JSVAL_TYPE_DOUBLE) { Nit: Brace goes on next line with multi-line conditonals. @@ +1962,5 @@ > + > + if (b.lhsTypes->getKnownTypeTag(cx) != JSVAL_TYPE_INT32 && > + b.lhsTypes->getKnownTypeTag(cx) != JSVAL_TYPE_DOUBLE && > + b.lhsTypes->getKnownTypeTag(cx) != JSVAL_TYPE_UNDEFINED) > + return false; These two |if|s should be braced. ::: js/src/ion/Lowering.cpp @@ +381,5 @@ > + if (ins->specialization() == MIRType_None) { > + JS_ASSERT(lhs->type() == MIRType_None); > + JS_NOT_REACHED("NYI: Cannot bounce to AddV because we need a resumepoint."); > + return false; > + } Since MAdd is separate from MAddGeneric, you can just remove this whole if case, as well as the assert below, and above say: JS_ASSERT(ins->specialization() == MIRType_Double). ::: js/src/ion/MIR.h @@ +1389,5 @@ > + > + virtual AliasSet getAliasSet() const { > + if (input()->type() >= MIRType_Object) > + return AliasSet::Store(AliasSet::Any); > + return AliasSet::None(); This instruction should never have an object as an input, it looks like - so it should be okay to just return AliasSet::None(). @@ +1773,5 @@ > + return AliasSet::Store(AliasSet::Any); > + } > +}; > + > +class MBinaryStringInstruction You can probably fold this into MConcat. I'm not immediately aware of any other binary string instructions. ::: js/src/ion/TypePolicy.h @@ +112,5 @@ > > +class BinaryStringPolicy : public BoxInputsPolicy > +{ > + public: > + void specializeInputs(MInstruction *ins, TypeAnalysis *analyzer); You will probably run into a rebase against the type analysis overhaul - luckily, you can just delete anything related to specializeInputs. ::: js/src/jsinterp.cpp @@ +5485,5 @@ > + res->setDouble(double(l) + double(r)); > + TypeScript::MonitorOverflow(cx, script, pc); > + } else { > + res->setInt32(sum); > + } This is probably a change we want to mirror on mozilla-central, to ease merge pain. We can do it in a follow-up bug, but it should happen. We might need to figure out how to factor script/pc first though. ::: js/src/vm/String.cpp @@ -275,1 @@ > js_ConcatStrings(JSContext *cx, JSString *left, JSString *right) Heh. More jstracer vestige.

Comment on attachment 587165 [details] [diff] [review] Fix recovery of pc & script for inlined functions. Review of attachment 587165 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsinferinlines.h @@ +593,5 @@ > TypeDynamicResult(cx, script, pc, Type::UnknownType()); > } > > /* static */ inline void > +TypeScript::GetPcScript(JSContext *cx, JSScript *&script, jsbytecode *&pc) Nit: we usually prefer non-references for outparams, i.e. JSScript **, jsbytecode **

https://hg.mozilla.org/projects/ionmonkey/rev/41a9e0d849ea https://hg.mozilla.org/projects/ionmonkey/rev/ee506186bc06