Closed Bug 661703 Opened 13 years ago Closed 13 years ago

ion: adding JSOP_ADD to IonMonkey in order to test LICM


(Core :: JavaScript Engine, defect)

Not set





(Reporter: ascheff, Assigned: ascheff)



(1 file, 4 obsolete files)

I want to start working on LICM and it will be helpful to have some simple integer arithmetic to make loops and hoist things out of them.
Does BITAND (already there) or other bitops work? They're a bit easier for now since they don't have weird cases.
I don't think it would be too hard to (temporarily) map JSOP_ADD to a crippled Op_Add that only works on integers, and doesn't check for or handle overflow.
That's what I'm doing
Attachment #537200 - Flags: review+
Attached file Added MAdd class (obsolete) —
Attachment #537202 - Flags: review+
Attached file Added MAdd class (obsolete) —
Sorry if the diff is a little messy, it reflects many changes that aren't actually changes (like a + new line followed by a - new line).  Please tell me if it's bad to have that in there.  I just don't know.
Attachment #537200 - Attachment is obsolete: true
Attachment #537249 - Flags: review?(dvander)
Attachment #537226 - Attachment is obsolete: true
Attachment #537202 - Attachment is obsolete: true
Attachment #537203 - Attachment is obsolete: true
Comment on attachment 537249 [details] [diff] [review]
diff for my addition of JSOP_ADD

Review of attachment 537249 [details] [diff] [review]:

Looks good, r=me with nits picked

::: js/src/ion/MIR.cpp
@@ +352,5 @@
> +MAdd::New(MInstruction *left, MInstruction *right)
> +{
> +    return new MAdd(left, right);
> +}
> +

Hrm, starting with this opcode let's start putting trivial ::New functions in the header.

::: js/src/ion/MIR.h
@@ +759,5 @@
> +    MAdd(MInstruction *left, MInstruction *right)
> +      : MBinaryInstruction(left, right)
> +    {
> +        //wont always be an int in the future
> +        setResultType(MIRType_Int32);

Just do setResultType(MIRType_Value). The TypeOracle and inference pass will change it to MIRType_Int32 for you.

@@ +767,5 @@
> +    static MAdd *New(MInstruction *left, MInstruction *right);
> +
> +    //not sure about this specialization stuff
> +    MIRType requiredInputType(size_t index) const {

This is fine. Explanation: During MIR construction, the TypeOracle tells the instruction what input types it should expect. After MIR construction, a type analysis pass may downgrade specializations if those expectations clearly won't match. For binary instructions, the specialization tells you the input type.
Attachment #537249 - Flags: review?(dvander) → review+
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.