We should OSR exit with Int52Overflow when we fail to make an Int52 where we expect one.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

In DFG::Graph::addShouldSpeculateMachineInt(), we check !hasExitSite(add, Int52Overflow) when determining whether it's ok to speculate that an operand is of type Int52 or not.  However, the Int52Rep code that converts a double to Int52 will OSR exit with exit kind BadType instead.  This renders the hasExitSite() check in addShouldSpeculateMachineInt() useless.  This patch fixes this by changing Int52Rep to OSR exit with exit kind Int52Overflow instead when it fails to convert a double to an Int52.

[Mark Lam]

Created attachment 269617 [details]
proposed patch.

[Mark Lam]

Created attachment 269619 [details]
x86_64 benchmark result.

Perf is neutral so far with this patch.  None of the "definitely"s are consistently repeatable on retries.

Though perf is neutral in ToT, this will fix a bug that causes a perf regression in LongSpider crypto-aes when the patch for https://bugs.webkit.org/show_bug.cgi?id=153019 lands.

[Filip Pizlo]

Comment on attachment 269617 [details]
proposed patch.

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

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:2217
> -            DFG_TYPE_CHECK(
> +            DFG_TYPE_CHECK_WITH_EXIT_KIND(Int52Overflow,
>                  JSValueRegs(), node->child1(), SpecInt52AsDouble,
>                  m_jit.branch64(
>                      JITCompiler::Equal, resultGPR,

Yuck!  But I get it.

[Mark Lam]

Thanks for the review.  Landed in r195488: <http://trac.webkit.org/r195488>.