685695 - IonMonkey: Implement JSOP_SUB

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

I need this to run a benchmark with subtraction in it.

Comment 1

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

Attachment: patch

Comment 2

[Sean Stangl [:sstangl]]

Comment on attachment 559299 [details] [diff] [review]
patch

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

::: js/src/ion/LIR-Common.h
@@ +475,2 @@
>  // Adds two integers, returning an integer value.
> +class LAddI : public LBinaryMath<0>

This might read more cleanly as "LBinaryMath<Temps(0)>" -- the current state requires readers to chase definitions.

::: js/src/ion/MIR.cpp
@@ +109,5 @@
> +      case MDefinition::Op_Add:
> +        ret.setNumber(lhs.toNumber() + rhs.toNumber());
> +        break;
> +      case MDefinition::Op_Sub:
> +        ret.setNumber(lhs.toNumber() - rhs.toNumber());

I don't see any place where overflow is handled -- likewise for Add and Mul.

Comment 3

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

> ::: js/src/ion/MIR.cpp
> @@ +109,5 @@
> > +      case MDefinition::Op_Add:
> > +        ret.setNumber(lhs.toNumber() + rhs.toNumber());
> > +        break;
> > +      case MDefinition::Op_Sub:
> > +        ret.setNumber(lhs.toNumber() - rhs.toNumber());
> 
> I don't see any place where overflow is handled -- likewise for Add and Mul.

Okay, I see how this actually works now - the result is a double so it won't overflow, setNumber may re-cast to int, but we don't allow the MIR type to change (by GVN, it's too late).

Comment 4

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

http://hg.mozilla.org/projects/ionmonkey/rev/4129e39cffcd