Bug 227195

Summary: Add a new pattern to instruction selector to utilize SMSUBL supported by ARM64
Product: WebKit Reporter: Yijia Huang <yijia_huang>
Component: JavaScriptCoreAssignee: Yijia Huang <yijia_huang>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, kdaveport7, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Yijia Huang
Reported 2021-06-20 10:53:13 PDT
...
Attachments
Patch (13.53 KB, patch)
2021-06-21 07:46 PDT, Yijia Huang 		no flags
DetailsFormatted DiffDiff
Patch (15.41 KB, patch)
2021-06-21 11:15 PDT, Yijia Huang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yijia Huang
Comment 1 2021-06-21 07:46:53 PDT
Created attachment 431859 [details] Patch
Keith Miller
Comment 2 2021-06-21 11:08:44 PDT
Comment on attachment 431859 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431859&action=review looks good you should probably remove your comments before we land this though. > Source/JavaScriptCore/b3/B3LowerToAir.cpp:2589 > + if (m_locked.contains(v->child(0)) || m_locked.contains(v->child(1))) // <---- keep this? Why? Per offline discussion, we need this in case we have a pattern that matches against the Mul(Something(...), Something(...)). > Source/JavaScriptCore/b3/B3LowerToAir.cpp:2604 > + if (m_locked.contains(v->child(0))) // <---- keep this? Why? Ditto. e.g. SExt32(Something(...)) > Source/JavaScriptCore/b3/testb3_2.cpp:1221 > + Value* addValue = root->appendNew<Value>(proc, Sub, Origin(), aValue, mulValue); nit: should probably be subValue.
Yijia Huang
Comment 3 2021-06-21 11:15:27 PDT
Created attachment 431882 [details] Patch
Keith Miller
Comment 4 2021-06-21 11:18:57 PDT
Comment on attachment 431882 [details] Patch r=me.
EWS
Comment 5 2021-06-21 11:56:07 PDT
Committed r279075 (238995@main): <https://commits.webkit.org/238995@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 431882 [details].
Note You need to log in before you can comment on or make changes to this bug.