[webkit-reviews] review granted: [Bug 204904] get_by_id ICs should have a structure history used to indicate when we should skip generating an IC : [Attachment 384977] patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Saam Barati <sbarati at apple.com>'s
request for review:
Bug 204904: get_by_id ICs should have a structure history used to indicate when
we should skip generating an IC
https://bugs.webkit.org/show_bug.cgi?id=204904

Attachment 384977: patch

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




--- Comment #5 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 384977
  --> https://bugs.webkit.org/attachment.cgi?id=384977
patch

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

r=me with nits.

> Source/JavaScriptCore/dfg/DFGGraph.h:1156
> +    HashSet<Node*> m_shouldSkipIC;

Why not adding this information in DFGNode itself?

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:999
> +	   jsValueResult(resultRegs, node, DataFormatJS);

`jsValueResult(resultRegs, node);` is OK.

> Source/JavaScriptCore/jit/JITPropertyAccess.cpp:509
> +	   callOperationWithProfile(bytecode.metadata(m_codeBlock),
operationGetByIdGeneric, resultVReg,
TrustedImmPtr(m_codeBlock->globalObject()), regT0,
TrustedImmPtr(ident->impl()));

Don't we need to do ArrayProfile in generic path?