Closed
Bug 676933
Opened 13 years ago
Closed 13 years ago
IonMonkey: implement BITNOT
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: h4writer, Assigned: h4writer)
Details
Attachments
(2 files, 4 obsolete files)
16.96 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
2.64 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
Implement JSOP_BITNOT in ionmonkey.
Assignee | ||
Comment 1•13 years ago
|
||
Assignee: general → hv1989
Attachment #551145 -
Flags: review?(dvander)
Comment on attachment 551145 [details] [diff] [review] implement JSOP_BITNOT Review of attachment 551145 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/IonLowering.cpp @@ +195,5 @@ > + > + JS_ASSERT(input->type() == MIRType_Int32); > + > + LUnaryBitOp *bitop = new LUnaryBitOp(op, useRegister(input)); > + return defineReuseInput(bitop, ins); I suspect ARM's version of this will look slightly different, wanting define() instead of defineReuseInput() since it tends to have separate src and dest registers. We don't have to do it now but maybe we should think about having an abstraction here, either tweaking lowerForALU or introducing a Lowering-x86-shared? (because we don't have enough classes in our hierarchy :) ::: js/src/ion/MIR.cpp @@ +375,5 @@ > +MUnaryBitwiseInstruction::infer(const TypeOracle::Unary &u) > +{ > + if (u.ival == MIRType_Object) > + specialization_ = MIRType_None; > + else { House style is: If one sub-block gets braced, all should get braces.\: if (x) { ... one liner ... } else { ... ... }
Attachment #551145 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 3•13 years ago
|
||
- added requested changes - added the folding of bitnot - I added lowerForALU(LInstructionHelper<1, 1, 0>, ...) - ...
Attachment #551145 -
Attachment is obsolete: true
Attachment #552189 -
Flags: review?(dvander)
Comment on attachment 552189 [details] [diff] [review] implement JSOP_BITNOT Review of attachment 552189 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, I didn't think of this before, but I think the unary abstraction does not buy us anything here. There's only one unary bitop so we might as well just call everything BitOp instead of UnaryBitOp. ::: js/src/ion/IonBuilder.cpp @@ +365,5 @@ > case JSOP_IFEQ: > return jsop_ifeq(JSOP_IFEQ); > > + case JSOP_BITNOT: > + return jsop_unarybitop(op); jsop_bitnot @@ +1465,5 @@ > return true; > } > > bool > +IonBuilder::jsop_unarybitop(JSOp op) jsop_bitnot, and remove op input ::: js/src/ion/LIR-Common.h @@ +247,2 @@ > // a 32-bit integer result as an output. > +class LUnaryBitOp : public LInstructionHelper<1, 1, 0> Rename to LBitNot. ::: js/src/ion/LOpcodes.h @@ +50,5 @@ > _(Value) \ > _(Parameter) \ > _(TableSwitch) \ > _(Goto) \ > + _(UnaryBitOp) \ Ditto. ::: js/src/ion/Lowering.cpp @@ +161,5 @@ > } > } > > bool > +LIRGenerator::lowerUnaryBitOp(JSOp op, MInstruction *ins) Rename to lowerBitNot, remove op input, use MBitNot. ::: js/src/ion/MIR.h @@ +942,5 @@ > + > + void infer(const TypeOracle::Unary &u); > +}; > + > +class MBitNot : public MUnaryBitwiseInstruction Fold MUnaryBitwiseInstruction into MBitNot.
Attachment #552189 -
Flags: review?(dvander)
Assignee | ||
Comment 6•13 years ago
|
||
Now without the unary/binary layer
Attachment #552189 -
Attachment is obsolete: true
Attachment #552209 -
Flags: review?(dvander)
Assignee | ||
Comment 7•13 years ago
|
||
Attachment #552192 -
Attachment is obsolete: true
Attachment #552192 -
Flags: review?(dvander)
Attachment #552211 -
Flags: review?(dvander)
Assignee | ||
Comment 8•13 years ago
|
||
Sry for the spam, but just saw I forget to "qrefresh" before uploading the cleanup patch.
Attachment #552211 -
Attachment is obsolete: true
Attachment #552211 -
Flags: review?(dvander)
Attachment #552225 -
Flags: review?(dvander)
Updated•13 years ago
|
Attachment #552209 -
Flags: review?(dvander) → review+
Comment on attachment 552225 [details] [diff] [review] Cleanup Review of attachment 552225 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/Lowering.cpp @@ +172,3 @@ > > + ReorderCommutative(&lhs, &rhs); > + return lowerForALU(new LBitOp(op), ins, lhs, rhs); Eventually we will have LBitOpIC or something which can handle arbitrary values on either side - so let's keep the old version of this function for now. (If the assert is firing, we should file a bug.)
Attachment #552225 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 10•13 years ago
|
||
http://hg.mozilla.org/projects/ionmonkey/rev/29bd198d1ad1 http://hg.mozilla.org/projects/ionmonkey/rev/3a68a95b215b
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•