Closed
Bug 839501
Opened 11 years ago
Closed 11 years ago
BaselineCompiler: Compile remaining missing opcodes
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bhackett1024, Unassigned)
References
Details
Attachments
(1 file)
23.04 KB,
patch
|
djvj
:
review+
|
Details | Diff | Splinter Review |
The attached patch compiles all remaining opcodes that JM handles but Baseline doesn't, so that the two compilers handle the exact same set of opcodes. The new ones are: JSOP_IN JSOP_GETXPROP JSOP_GETINTRINSIC JSOP_CALLINTRINSIC JSOP_INSTANCEOF JSOP_TYPEOF JSOP_TYPEOFEXPR Most of these need ICs, though GETXPROP, CALLINTRINSIC and TYPEOFEXPR can reuse ICs for other ops.
Attachment #711841 -
Flags: review?(kvijayan)
Comment 1•11 years ago
|
||
Comment on attachment 711841 [details] [diff] [review] patch Review of attachment 711841 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/BaselineIC.cpp @@ +2354,5 @@ > + RootedObject obj(cx, &objValue.toObject()); > + > + JSBool cond = false; > + if (!OperatorIn(cx, key, obj, &cond)) > + return false; I think this is an existing issue with Ion, but OperatorIn is slightly divergent from the interpreter implementation. Interpreter uses |ValueToId| defined in jsatominlines.h, while OperatorIn uses |FetchElementId| defined in jsinterpinlines.h. The implementations of |ValueToId| and |FetchElementId| are basically identical, despite being in two different header files. I think it'd be good to refactor this now and land it on m-i before proceeding with the changes in baseline. @@ +2573,5 @@ > + > +bool > +ICGetIntrinsic_Constant::Compiler::generateStubCode(MacroAssembler &masm) > +{ > + masm.loadValue(Address(BaselineStubReg, offsetof(ICGetIntrinsic_Constant, value_)), R0); Nit: make ICGetIntrinsic_Contant::offsetOfValue() method for this. Keeps it in line with convention on other stubs. @@ +3848,5 @@ > + MutableHandleValue res) > +{ > + FallbackICSpew(cx, stub, "InstanceOf"); > + > + if (!rhs.isObject()) { Would be good to refactor jsinterp.cpp's handling of JSOP_INSTANCEOF first into a reusable method, and then have the fallback stub reuse that. There are slight divergences creeping in here as well (e.g. here it's "!rhs.isObject()", in jsinterp.cpp it's "rhs.isPrimitive()"). Good to refactor and land on m-i as with above.
Attachment #711841 -
Flags: review?(kvijayan) → review+
Reporter | ||
Comment 2•11 years ago
|
||
Filed bug 839526 for general interpreter related cleanup. https://hg.mozilla.org/projects/ionmonkey/rev/d62ba56adc27
Reporter | ||
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•