1028698 - IonMonkey: Implement Abs Recover Instruction

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Rémi WENG]

Implement Abs in js/src/jit/Recover.cpp.
See Bug 1003801 for explanation.

Comment 1

[Rémi WENG]

Attachment: bug1028698.patch

Comment 2

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8446720 [details] [diff] [review]
bug1028698.patch

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

This sounds good.
A few last thing and this can land. ;)

::: js/src/jit-test/tests/ion/dce-with-rinstructions.js
@@ +385,5 @@
>      return i;
>  }
> +var uceFault_abs = eval(uneval(uceFault).replace('uceFault', 'uceFault_abs'));
> +function rabs_number(i) {
> +    var x = Math.abs(-42);

Use "i - 42" instead, such as we cannot remove the Math.abs function in any way while compiling this function.

::: js/src/jit/Recover.h
@@ +330,5 @@
>          return 1;
>      }
>      bool recover(JSContext *cx, SnapshotIterator &iter) const;
>  };
> +class RAbs MOZ_FINAL : public RInstruction

nit: add an extra new line before the class definition.

::: js/src/jsmath.cpp
@@ +87,5 @@
>      JS_ResolveStub,
>      JS_ConvertStub
>  };
>  bool
> +js_math_abs_handle(JSContext *cx, js::HandleValue v, js::MutableHandleValue r)

nit: same here, before bool.

Comment 3

[Rémi WENG]

Attachment: bug1028698.patch

Test the patch whether extra new lines are present or not

Comment 4

[Rémi WENG]

Attachment: bug1028698.patch

It is very weird that there are not extra lines in the diff because in details, we can see that I added these lines. An explanation ?

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Rémi WENG from comment #4)
> Created attachment 8447228 [details] [diff] [review]
> bug1028698.patch
> 
> It is very weird that there are not extra lines in the diff because in
> details, we can see that I added these lines. An explanation ?

Is there an unnumbered line?

Comment 6

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8447228 [details] [diff] [review]
bug1028698.patch

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

Awesome :)
I will send this patch to try, and land it for you.

Do you want to try other kind of Bugs?

Comment 7

[Nicolas B. Pierron [:nbp]]

https://tbpl.mozilla.org/?tree=Try&rev=a6295f66213a

Comment 8

[Rémi WENG]

(In reply to Nicolas B. Pierron [:nbp] from comment #5)
> (In reply to Rémi WENG from comment #4)
> > Created attachment 8447228 [details] [diff] [review]
> > bug1028698.patch
> > 
> > It is very weird that there are not extra lines in the diff because in
> > details, we can see that I added these lines. An explanation ?
> 
> Is there an unnumbered line?

It seems not.

> Do you want to try other kind of Bugs?

With pleasure :)

Comment 9

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ed7bd8ab1f3

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/4ed7bd8ab1f3