940642 - IonMonkey: Incorrect result for multiplication when also calling imul

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jesse Ruderman]

function sprod(x, y) {
    var iprod = Math.imul(x | 0, y | 0);
    var fprod = (x | 0) * (y | 0);
    return iprod + fprod;
}
assertEq(sprod(2, 2), 8);
assertEq(sprod(0x10000, 0x10000), 0x100000000);


With --ion-eager:
c.js:7:0 Error: Assertion failed: got 0, expected 4294967296


The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/b9c4a9483492
user:        Tom Schuster
date:        Fri Jan 04 00:10:19 2013 +0100
summary:     Bug 822436 - IonMonkey: Inline Math.imul. r=h4writer

Comment 1

[Tom S [:evilpie]]

Attachment: imul

Comment 2

[Hannes Verschore [:h4writer]]

Comment on attachment 8336773 [details] [diff] [review]
imul

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

::: js/src/jit/MIR.h
@@ +4000,5 @@
> +
> +        MMul *mul = ins->toMul();
> +        if (canBeNegativeZero_ != mul->canBeNegativeZero() ||
> +            mode_ != mul->mode())
> +            return false;

I would split it as following:

if (canBeNegativeZero_ != mul->canBeNegativeZero())
    return false;

if (mode_ != mul->mode())
    return false;

@@ +4002,5 @@
> +        if (canBeNegativeZero_ != mul->canBeNegativeZero() ||
> +            mode_ != mul->mode())
> +            return false;
> +
> +        return congruentIfOperandsEqual(ins);

MBinaryInstruction::congruentTo()

That will also change to order of the operands if that is deemed better.

Comment 3

[Tom S [:evilpie]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/883577706b74

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/883577706b74