Closed Bug 1028698 Opened 8 years ago Closed 8 years ago

IonMonkey: Implement Abs Recover Instruction

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: contact, Assigned: contact, Mentored)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Implement Abs in js/src/jit/Recover.cpp.
See Bug 1003801 for explanation.
Assignee: nobody → wengremi
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attached patch bug1028698.patch (obsolete) — Splinter Review
Attachment #8446720 - Flags: review?(nicolas.b.pierron)
Blocks: 1003801
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.
Attachment #8446720 - Flags: review?(nicolas.b.pierron) → feedback+
Attached patch bug1028698.patch (obsolete) — Splinter Review
Test the patch whether extra new lines are present or not
Attachment #8446720 - Attachment is obsolete: true
Attached patch bug1028698.patchSplinter Review
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 ?
Attachment #8447220 - Attachment is obsolete: true
Attachment #8447228 - Flags: review?(nicolas.b.pierron)
(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 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?
Attachment #8447228 - Flags: review?(nicolas.b.pierron) → review+
(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 :)
https://hg.mozilla.org/mozilla-central/rev/4ed7bd8ab1f3
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.