[webkit-reviews] review granted: [Bug 228674] [JSC] branchTest8 should not emit tst for Zero/NonZero cases : [Attachment 434686] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 228674: [JSC] branchTest8 should not emit tst for Zero/NonZero cases
https://bugs.webkit.org/show_bug.cgi?id=228674

Attachment 434686: Patch

https://bugs.webkit.org/attachment.cgi?id=434686&action=review




--- Comment #3 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 434686
  --> https://bugs.webkit.org/attachment.cgi?id=434686
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.