Bug 228674

Summary: [JSC] branchTest8 should not emit tst for Zero/NonZero cases
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch mark.lam: review+, ews-feeder: commit-queue-

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>