1011540 - IonMonkey: Implement Mul Recover Instruction

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Nicolas B. Pierron [:nbp]]

Implement RMul in js/src/jit/Recover.cpp.
See Bug 1003801 comment 0 for explanation.

Comment 1

[Sankha Narayan Guria [:sankha]]

Attachment: patchv1

Comment 2

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8424464 [details] [diff] [review]
patchv1

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

(Canceled review, fix RSub and this patch too before fixing other bugs)

Comment 3

[Mike Hoye [:mhoye]]

Can I assign this bug to Sankha if he's working on it?

Comment 4

[Nicolas B. Pierron [:nbp]]

(In reply to Mike Hoye [:mhoye] from comment #3)
> Can I assign this bug to Sankha if he's working on it?

Bug 1011539 is not finished yet, and I don't know if he is still working on these bugs.

Comment 5

[Sankha Narayan Guria [:sankha]]

Attachment: patchv2

Comment 6

[Sankha Narayan Guria [:sankha]]

Attachment: iongraph output

This shows the iongraph output for RMul instruction in the rmul-float function. The mul instructions are coloured in gray like resume points.

Comment 7

[Sankha Narayan Guria [:sankha]]

Attachment: patchv2

Comment 8

[Nicolas B. Pierron [:nbp]]

Comment on attachment 8435736 [details] [diff] [review]
patchv2

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

Nice :)

Fix the two cases and check your patch on Try.
If you do not have commit access yet, upload a new patch with the recommended fixes, such as Sheriff/I can land the definitive versions.

::: js/src/jit-test/tests/ion/dce-with-rinstructions.js
@@ +195,5 @@
> +var uceFault_mul_number = eval(uneval(uceFault).replace('uceFault', 'uceFault_mul_number'));
> +function rmul_number(i) {
> +    var x = 1 * i;
> +    if (uceFault_mul_number(i) || uceFault_mul_number(i))
> +        assertEq(x, 99  /* = 1 * 99 */);

Take another value than 1, as it is possible that we just remove this multiplication (or should remove it).

::: js/src/jit/MIR.h
@@ +4406,5 @@
>      Mode mode() const { return mode_; }
> +
> +    bool writeRecoverData(CompactBufferWriter &writer) const;
> +    bool canRecoverOnBailout() const {
> +        return specialization_ != MIRType_None;

Use a similar condition as the getAliasSet method of this MIR (in this case it is inherited)

Comment 9

[Sankha Narayan Guria [:sankha]]

Attachment: patch v3

Putting the r+ from previous patch.

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9e074affd0a3

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9e074affd0a3