[webkit-reviews] review granted: [Bug 174355] [JSC] Optimize Map iteration with intrinsic : [Attachment 318871] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Yusuke Suzuki
<utatane.tea at gmail.com>'s request for review:
Bug 174355: [JSC] Optimize Map iteration with intrinsic
https://bugs.webkit.org/show_bug.cgi?id=174355

Attachment 318871: Patch

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




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

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

r=me

> Source/JavaScriptCore/builtins/MapPrototype.js:83
> +	   if (bucket == @sentinelMapBucket)

let's use === instead of ==

> Source/JavaScriptCore/builtins/SetPrototype.js:73
> +	   if (bucket == @sentinelSetBucket)

Let's use === instead of ==.
Also, do all of our tiers properly handle comparing cells to each other that
aren't strings/symbols?

> Source/JavaScriptCore/dfg/DFGClobberize.h:152
> +    case CompareEqPtr:

Nice catch!

> Source/JavaScriptCore/dfg/DFGNode.h:2541
> +	   return m_opInfo.as<UseKind>();

Style nit: This feels like a weird override of UseKind, and I think it's a
little confusing to see it used in this context. Why not just create your own
separate enum for this purpose?