[webkit-reviews] review granted: [Bug 194953] DFGBytecodeParser should not declare that a node won't clobberExit if DFGFixupPhase can later declare it does clobberExit : [Attachment 362738] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Robin Morisset
<rmorisset at apple.com>'s request for review:
Bug 194953: DFGBytecodeParser should not declare that a node won't clobberExit
if DFGFixupPhase can later declare it does clobberExit
https://bugs.webkit.org/show_bug.cgi?id=194953

Attachment 362738: Patch

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




--- Comment #3 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 362738
  --> https://bugs.webkit.org/attachment.cgi?id=362738
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:14
> +	   Otherwise we are likely to hit validation failure after fixup.

instead of "likely" I think you should describe precisely when/why this
happens.

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:7216
> +	   m_exitOK = false; // PutByVal and PutByValDirect must be treated as
if they clobber exit state, since FixupPhase may make them generic.

This one isn't actually needed since PutByVal always clobbers exit unless it's
in ForceOSR exit mode. And if it's in that mode, I don't think we can ever
refine it to be wider? I'm ok with keeping this here conservatively, but this
isn't the right comment to have.