Bug 149713 – DFG peephole does not have effect on CompareEq / CompareStrictEq because of MovHint

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: JavaScriptCore

[Yusuke Suzuki]

Currently, we have op_jless, op_jgreater etc. But we don't have op_jeq.
Instead we compile the following

function eq(a, b)
{
    if (a === b)
        return true;
    return false;
}

to

eq#CU1c7h:[0x7fb57e6dd268->0x7fb57e46c200, NoneFunctionCall, 17]: 17 m_instructions; 136 bytes; 3 parameter(s); 8 callee register(s); 6 variable(s)
[   0] enter             
[   1] get_scope         loc3
[   3] mov               loc4, loc3
[   6] eq                loc6, arg1, arg2
[  10] jfalse            loc6, 5(->15)
[  13] ret               True(const1)
[  15] ret               False(const2)

So, in DFG, between op_eq and op_jfalse, we have MovHint to store the result of op_eq.
As a result, detectPeepHoleBranch() never detects peep hole for that case.

[Filip Pizlo]

When and if we implement a fix that allows the peephole detection to "skip" the MovHint, we should check how much of a speed-up it is.

If it is not a speed-up, then instead of writing a patch that extends the peephole to skip MovHint, we should write a patch that removes all code for those peepholes that are currently dead because of the MovHint.  That peephole code is a pain to maintain and it's possible that it buys us nothing.

[Yusuke Suzuki]

(In reply to comment #1)
> When and if we implement a fix that allows the peephole detection to "skip"
> the MovHint, we should check how much of a speed-up it is.
> 
> If it is not a speed-up, then instead of writing a patch that extends the
> peephole to skip MovHint, we should write a patch that removes all code for
> those peepholes that are currently dead because of the MovHint.  That
> peephole code is a pain to maintain and it's possible that it buys us
> nothing.

Right. This peep hole optimization is easily broken because the assumption the optimization rely on is fragile.
If it does not provide significant performance improvement, dropping these code is better.

[Saam Barati]

Yeah, I remember running into not being able
to get peephole optimization to kick in because
of MovHints (that's why I asked you to upload
a test for that, because I thought you found
a way for it to kick in).

[Yusuke Suzuki]

*** This bug has been marked as a duplicate of bug 177100 ***