Bug 228674 - [JSC] branchTest8 should not emit tst for Zero/NonZero cases
Summary: [JSC] branchTest8 should not emit tst for Zero/NonZero cases
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-30 20:11 PDT by Yusuke Suzuki
Modified: 2021-07-30 23:34 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.47 KB, patch)
2021-07-30 20:14 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (2.65 KB, patch)
2021-07-30 20:19 PDT, Yusuke Suzuki
mark.lam: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2021-07-30 20:11:42 PDT
[JSC] branchTest8 should not emit tst for Zero/NonZero cases
Comment 1 Yusuke Suzuki 2021-07-30 20:14:51 PDT
Created attachment 434685 [details]
Patch
Comment 2 Yusuke Suzuki 2021-07-30 20:19:56 PDT
Created attachment 434686 [details]
Patch
Comment 3 Mark Lam 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.
Comment 4 Yusuke Suzuki 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.
Comment 5 Yusuke Suzuki 2021-07-30 21:46:38 PDT
Committed r280508 (240140@main): <https://commits.webkit.org/240140@main>
Comment 6 Radar WebKit Bug Importer 2021-07-30 21:47:27 PDT
<rdar://problem/81355381>