661703 - ion: adding JSOP_ADD to IonMonkey in order to test LICM

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andrew Scheff]

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.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Does BITAND (already there) or other bitops work? They're a bit easier for now since they don't have weird cases.

Comment 2

[Andrew Drake [:adrake]]

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.

Comment 3

[Andrew Scheff]

That's what I'm doing

Comment 4

[Andrew Scheff]

Attachment: Added a simplified JSOP_ADD to IonBuilder

Comment 5

[Andrew Scheff]

Attachment: Added MAdd class

Comment 6

[Andrew Scheff]

Attachment: Added MAdd class

Comment 7

[Andrew Scheff]

Attachment: The correct attachment, ignore the previous 3

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.

Comment 8

[Andrew Scheff]

Attachment: diff for my addition of JSOP_ADD

Comment 9

[David Anderson [:dvander] - inactive, e-mail if emergency]

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 @@
> +    INSTRUCTION_HEADER(Add);
> +    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.

Comment 10

[Andrew Scheff]

http://hg.mozilla.org/users/danderson_mozilla.com/ionmonkey/rev/035604ebd262