[webkit-reviews] review granted: [Bug 181466] Support MultiGetByOffset in the DFG : [Attachment 331542] patch

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Saam Barati
<sbarati at apple.com>'s request for review:
Bug 181466: Support MultiGetByOffset in the DFG
https://bugs.webkit.org/show_bug.cgi?id=181466

Attachment 331542: patch

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




--- Comment #17 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 331542
  --> https://bugs.webkit.org/attachment.cgi?id=331542
patch

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

r=me with some comments.

> Source/JavaScriptCore/dfg/DFGGraph.h:980
> +	   // We want to ensure we get polyvariant profiling data from the
GetById. This allows
> +	   // the same get_by_id inlined into two separate functions to get
independent profiling data.

It seems like we could still profile in the MultiGetByOffset when the the
origin is from an inlined source. Maybe file a bug?

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4371
> +		   m_jit.load32(JITCompiler::Address(baseGPR,
JSCell::structureIDOffset()), resultGPR);
> +		   JITCompiler::JumpList match;
> +		   for (size_t i = 0; i < structures.size() - 1; ++i) {
> +		       match.append(
> +			   m_jit.branchWeakStructure(JITCompiler::Equal,
resultGPR, structures[i]));
> +		   }
> +		   next = m_jit.branchWeakStructure(JITCompiler::NotEqual,
resultGPR, structures.last());
> +		   match.link(&m_jit);

Couldn't this use BinarySwitch?