RESOLVED FIXED 228674
[JSC] branchTest8 should not emit tst for Zero/NonZero cases
https://bugs.webkit.org/show_bug.cgi?id=228674
Summary [JSC] branchTest8 should not emit tst for Zero/NonZero cases
Yusuke Suzuki
Reported 2021-07-30 20:11:42 PDT
[JSC] branchTest8 should not emit tst for Zero/NonZero cases
Attachments
Patch (2.47 KB, patch)
2021-07-30 20:14 PDT, Yusuke Suzuki
no flags
Patch (2.65 KB, patch)
2021-07-30 20:19 PDT, Yusuke Suzuki
mark.lam: review+
ews-feeder: commit-queue-
Yusuke Suzuki
Comment 1 2021-07-30 20:14:51 PDT
Yusuke Suzuki
Comment 2 2021-07-30 20:19:56 PDT
Mark Lam
Comment 3 2021-07-30 21:23:00 PDT
Comment on attachment 434686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434686&action=review r=me > Source/JavaScriptCore/ChangeLog:9 > + unnecessary. This is because the mask is truncated into 8bit, which /unnecessary/unnecessarily/. > Source/JavaScriptCore/ChangeLog:14 > + This patch removes this unnecessary truncation since Zero/NonZero does > + not care high bits of mask in branchTest8. I think it's worth pointing out that this is ok because the ResultCondition version of mask8OnCondition() is always used to generate a mask that is only used against a value that is loaded with MacroAssemblerHelpers::load8OnCondition(). For Zero/NonZero conditions, load8OnCondition() will always zero fill the upper bytes. The 0 filled upper bytes will not affect the result of a branch on zero or branch on not zero.
Yusuke Suzuki
Comment 4 2021-07-30 21:45:04 PDT
Comment on attachment 434686 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434686&action=review >> Source/JavaScriptCore/ChangeLog:9 >> + unnecessary. This is because the mask is truncated into 8bit, which > > /unnecessary/unnecessarily/. Fixed. >> Source/JavaScriptCore/ChangeLog:14 >> + not care high bits of mask in branchTest8. > > I think it's worth pointing out that this is ok because the ResultCondition version of mask8OnCondition() is always used to generate a mask that is only used against a value that is loaded with MacroAssemblerHelpers::load8OnCondition(). For Zero/NonZero conditions, load8OnCondition() will always zero fill the upper bytes. The 0 filled upper bytes will not affect the result of a branch on zero or branch on not zero. Sounds good. Added.
Yusuke Suzuki
Comment 5 2021-07-30 21:46:38 PDT
Radar WebKit Bug Importer
Comment 6 2021-07-30 21:47:27 PDT
Note You need to log in before you can comment on or make changes to this bug.