Closed Bug 670810 Opened 13 years ago Closed 13 years ago

IonMonkey: implement BITOR/BITXOR opcode

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: h4writer, Assigned: h4writer)

Details

Attachments

(1 file, 1 obsolete file)

(now something rather easy ;))

I just took the code used for BitAnd and transformed it into BitOr.
(Because no real code gets executed, most is the same between BitAnd and BitOr)
Attached patch Patch (obsolete) — Splinter Review
Attachment #545304 - Flags: review?(dvander)
I'm checking in the codegen patches in a few hours - do you want to wait for that and then do the codegen part of this opcode as well?
Assignee: general → hv1989
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(In reply to comment #2)
> I'm checking in the codegen patches in a few hours - do you want to wait for
> that and then do the codegen part of this opcode as well?

yeah sure
Attachment #545304 - Flags: review?(dvander) → review?(adrake)
Comment on attachment 545304 [details] [diff] [review]
Patch

Review of attachment 545304 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! Just one little nit:

::: js/src/ion/IonBuilder.cpp
@@ +380,4 @@
>          return jsop_binary(op);
>  
>        case JSOP_ADD:
>        	return jsop_binary(op);

Nit: I'd probably merge these two cases while we're here.
Attachment #545304 - Flags: review?(adrake) → review+
(In reply to comment #4)
> Comment on attachment 545304 [details] [diff] [review] [review]
> Patch
> 
> Review of attachment 545304 [details] [diff] [review] [review]:
> -----------------------------------------------------------------
> 
> Looks good! Just one little nit:
> 
> ::: js/src/ion/IonBuilder.cpp
> @@ +380,4 @@
> >          return jsop_binary(op);
> >  
> >        case JSOP_ADD:
> >        	return jsop_binary(op);
> 
> Nit: I'd probably merge these two cases while we're here.

I deliberate didn't do that. Because JSOP_ADD isn't a binary operation. JSOP_ADD is just temporary hacked together to do some testing. So I think it don't make sense to merge those...
Attached patch PatchSplinter Review
Bitwise OR and XOR are implemented in this patch.
There was also a typo in bitwise AND I fixed.
Attachment #545304 - Attachment is obsolete: true
Attachment #545844 - Flags: review?(adrake)
Summary: IonMonkey: implement BITOR opcode → IonMonkey: implement BITOR/BITXOR opcode
Comment on attachment 545844 [details] [diff] [review]
Patch

Review of attachment 545844 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, two little things:

::: js/src/ion/IonLowering.cpp
@@ +217,5 @@
> +
> +bool
> +LIRGenerator::visitBitXOr(MBitXOr *ins)
> +{
> +    return doBitOp(JSOP_BITOR, ins);

JSOP_BITXOR

::: js/src/ion/LIR-Common.h
@@ +229,5 @@
>          setOperand(0, left);
>          setOperand(1, right);
>      }
> +
> +    JSOp op() {

This should be called something other than op(), LIR_HEADER(opcode) defines:

    Opcode op() const {                                                     \
        return LInstruction::LOp_##opcode;                                  \
    }                                                                       \
Attachment #545844 - Flags: review?(adrake) → review+
http://hg.mozilla.org/users/danderson_mozilla.com/ionmonkey/rev/42375f98039f
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.