[webkit-reviews] review granted: [Bug 171390] B3::FoldPathConstants does not consider the fall through case for Switch : [Attachment 308444] patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Saam Barati <sbarati at apple.com>'s
request for review:
Bug 171390: B3::FoldPathConstants does not consider the fall through case for
Switch
https://bugs.webkit.org/show_bug.cgi?id=171390

Attachment 308444: patch

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




--- Comment #5 from Filip Pizlo <fpizlo at apple.com> ---
Comment on attachment 308444
  --> https://bugs.webkit.org/attachment.cgi?id=308444
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308444&action=review

R=me without that if.

>> Source/JavaScriptCore/b3/B3FoldPathConstants.cpp:109
>> +			targetUses.add(switchValue->fallThrough(block),
0).iterator->value++;
> 
> If we wanted to, we could be a bit niftier here:
> We could check if any target is zero, and if so, we could check if
targetUses(fall through) === 0, and if both are true, we could say that in the
default block, the switch value is nonzero (obviously only if default block has
a single predecessor). I'm not sure if it's worth bothering though.

VALIDATE(value->as<SwitchValue>()->hasFallThrough(valueOwner.get(value)), ("At
", *value));

Therefore, you don't need to check if hasFallThrough().  It must
hasFallThrough().