789420 - IonMonkey: Incorrectly optimizing bitwise ops

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

The 2 testcases below fail on revision d16c4404e8c4 with --ion-eager or --no-jm.

(1) Folding 0 & x to 0 no longer calls the valueOf method (DCE eliminates the TruncateToInt32 instruction):

function g(x, y) {
    return 0 & y;
}
var c = 0;
function f() {
    for (var i=0; i<100; i++) {
        g(i, i);
        g(i, {valueOf: function() { c++; return 0.1; }});
    }
}
f();
assertEq(c, 100);

(2) Unused bitop + TruncateToInt32 is eliminated:

function g(x, y) {
    333 | y;
}
var c = 0;
function f() {
    for (var i=0; i<100; i++) {
        g(i, i);
        g(i, {valueOf: function() { c++; return 0.1; }});
    }
}
f();
assertEq(c, 100);

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

Don't use int32 specialization if an operand may be an object.

Comment 2

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

This bug is a little concerning because it feels like we're actually missing a guard or something. It's unusual that we'd have to check whether something is "maybe an object".

Maybe the real problem is that TruncateToInt32 is implicitly guarding but it's not a guard. Or that it has no type policy so it never inserts an unbox or anything.

(Maybe this is why CS doesn't do DCE.)

Comment 3

[Jan de Mooij [:jandem]]

(In reply to David Anderson [:dvander] from comment #2)
> This bug is a little concerning because it feels like we're actually missing
> a guard or something. It's unusual that we'd have to check whether something
> is "maybe an object".

"maybe an object" is just "definitely primitive", and imho that's not very different from the "definitely int32 or definitely double" etc checks we do elsewhere.

> 
> Maybe the real problem is that TruncateToInt32 is implicitly guarding but
> it's not a guard. Or that it has no type policy so it never inserts an unbox
> or anything.

Yeah I thought about this too. An advantage of my approach is that it doesn't require marking instructions as guard and it can handle objects without bailing out. I don't have a very strong opinion on this though, let me know what you prefer.

Comment 4

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c1128d7e03ec

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c1128d7e03ec