[webkit-reviews] review granted: [Bug 227138] Add a new pattern to B3ReduceStrength based on Bug 226984 : [Attachment 431779] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Jun 18 10:16:35 PDT 2021
Filip Pizlo <fpizlo at apple.com> has granted Yijia Huang
<yijia_huang at apple.com>'s request for review:
Bug 227138: Add a new pattern to B3ReduceStrength based on Bug 226984
https://bugs.webkit.org/show_bug.cgi?id=227138
Attachment 431779: Patch
https://bugs.webkit.org/attachment.cgi?id=431779&action=review
--- Comment #5 from Filip Pizlo <fpizlo at apple.com> ---
Comment on attachment 431779
--> https://bugs.webkit.org/attachment.cgi?id=431779
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.
More information about the webkit-reviews
mailing list