Bug 227138 – Add a new pattern to B3ReduceStrength based on Bug 226984

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yijia Huang]

...

[Yijia Huang]

Created attachment 431754 [details]
Patch

[Yijia Huang]

Created attachment 431755 [details]
Patch

[Yijia Huang]

Created attachment 431758 [details]
Patch

[Yijia Huang]

Created attachment 431779 [details]
Patch

[Filip Pizlo]

Comment on attachment 431779 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=431779&action=review

I think this is correct, but I'd make the two small changes I suggest.

> Source/JavaScriptCore/b3/B3ReduceStrength.cpp:1044
> +                && !(m_value->child(0)->child(0)->hasInt())

I don't think you need this condition.  Your transformation is still correct even if that value is a constant.

If it was a constant then it would probably also get constant-folded, but you don't have to worry about that.  It's better to just have only the checks you really need.

> Source/JavaScriptCore/b3/B3ReduceStrength.cpp:1048
> +                int64_t const1 = m_value->child(0)->child(1)->asInt();
> +                uint64_t const2 = m_value->child(1)->asInt();

Can we give these better names?  Like, I'd use shiftAmount instead of const1.  And I'd use mask for const2.

[Yijia Huang]

Created attachment 431780 [details]
Patch

[EWS]

Committed r279042 (238962@main): <https://commits.webkit.org/238962@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 431780 [details].