[webkit-reviews] review granted: [Bug 210882] Fix OSR exiting/iterator object checks in for-of bytecodes : [Attachment 397358] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 210882: Fix OSR exiting/iterator object checks in for-of bytecodes
https://bugs.webkit.org/show_bug.cgi?id=210882

Attachment 397358: Patch

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




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

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

r=me

> Source/JavaScriptCore/ChangeLog:19
> +	   Finally, this patch makes a small optimization where we just
ArithBitOr the
> +	   iterator's closed state (index == -1) and index is out of bounds. We
can't

Is this actually faster?

> Source/JavaScriptCore/ChangeLog:20
> +	   do a CompareBelow check because the index is effectively an int33_t.

int33 => int32_t

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:255
> -	   bool isDirect, unsigned intructionSize, ECMAMode);
> +	   bool isDirect, BytecodeIndex osrExitIndex, ECMAMode);

nice

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:6887
> +		       Node* doneIndex = jsConstant(jsNumber(-1));

same comment as your original patch: can we make this a constant somewhere and
use it by name? We hardcode -1 in a lot of places and it's not the best
abstraction