Bug 227138

Summary: Add a new pattern to B3ReduceStrength based on Bug 226984
Product: WebKit Reporter: Yijia Huang <yijia_huang>
Component: JavaScriptCoreAssignee: Yijia Huang <yijia_huang>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch none

Description Yijia Huang 2021-06-17 12:10:46 PDT
...
Comment 1 Yijia Huang 2021-06-17 22:23:32 PDT
Created attachment 431754 [details]
Patch
Comment 2 Yijia Huang 2021-06-17 22:25:42 PDT
Created attachment 431755 [details]
Patch
Comment 3 Yijia Huang 2021-06-17 23:31:51 PDT
Created attachment 431758 [details]
Patch
Comment 4 Yijia Huang 2021-06-18 10:03:51 PDT
Created attachment 431779 [details]
Patch
Comment 5 Filip Pizlo 2021-06-18 10:16:35 PDT
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.
Comment 6 Yijia Huang 2021-06-18 10:38:40 PDT
Created attachment 431780 [details]
Patch
Comment 7 EWS 2021-06-18 11:25:57 PDT
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].