[webkit-reviews] review granted: [Bug 213141] CheckIsConstant should not use BadCache exit kind : [Attachment 401812] Patch

[bugzilla-daemon at webkit.org]

Yusuke Suzuki <ysuzuki at apple.com> has granted Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 213141: CheckIsConstant should not use BadCache exit kind
https://bugs.webkit.org/show_bug.cgi?id=213141

Attachment 401812: Patch

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




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

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

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:9676
> -	   speculationCheck(BadCache, regs, node->child1(),
m_jit.branch64(JITCompiler::NotEqual, regs.gpr(),
TrustedImm64(JSValue::encode(node->constant()->value()))));
> +	   speculationCheck(BadConstantValue, regs, node->child1(),
m_jit.branch64(JITCompiler::NotEqual, regs.gpr(),
TrustedImm64(JSValue::encode(node->constant()->value()))));
>  #else
> -	   speculationCheck(BadCache, regs, node->child1(),
m_jit.branch32(JITCompiler::NotEqual, regs.tagGPR(),
TrustedImm32(node->constant()->value().tag())));
> -	   speculationCheck(BadCache, regs, node->child1(),
m_jit.branch32(JITCompiler::NotEqual, regs.payloadGPR(),
TrustedImm32(node->constant()->value().payload())));
> +	   speculationCheck(BadConstantValue, regs, node->child1(),
m_jit.branch32(JITCompiler::NotEqual, regs.tagGPR(),
TrustedImm32(node->constant()->value().tag())));
> +	   speculationCheck(BadConstantValue, regs, node->child1(),
m_jit.branch32(JITCompiler::NotEqual, regs.payloadGPR(),
TrustedImm32(node->constant()->value().payload())));

One question. This changes BadCache -> BadConstantValue. So, we also need to
check `hasExitSite(codeOrigin, BadCache)` things too.
For example, Graph::canOptimizeStringObjectAccess has

  1775 bool Graph::canOptimizeStringObjectAccess(const CodeOrigin& codeOrigin)
  1776 {
  1777	   if (hasExitSite(codeOrigin, BadCache) || hasExitSite(codeOrigin,
BadConstantCache))
  1778	       return false;

this code. PutById etc. has,

  1847	       case PutById:
  1848	       case PutByIdFlush:
  1849	       case PutByIdDirect: {
  1850		   if (node->child1()->shouldSpeculateCellOrOther()
  1851		       && !m_graph.hasExitSite(node->origin.semantic, BadType)
  1852		       && !m_graph.hasExitSite(node->origin.semantic, BadCache)
  1853		       && !m_graph.hasExitSite(node->origin.semantic,
BadIndexingType)
  1854		       && !m_graph.hasExitSite(node->origin.semantic,
ExoticObjectMode)) {

this code. And there are many this type of codes. Can you ensure that they are
correct after this change?