[webkit-reviews] review granted: [Bug 196639] Clean up Int52 code and some bugs in it : [Attachment 366907] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 9 00:24:57 PDT 2019

Yusuke Suzuki <ysuzuki at apple.com> has granted Saam Barati <sbarati at apple.com>'s
request for review:
Bug 196639: Clean up Int52 code and some bugs in it

Attachment 366907: patch


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

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


> Source/JavaScriptCore/ChangeLog:16
> +	   However, this got confusing because we reused SpecInt32Only both for
> +	   representations and Int52 representations. This actually lead to
some bugs.

I like this idea, it simplifies shouldSpeculateInt52 flow too, and now we do
not need to bother whether we should include SpecInt32Only in
shouldSpeculateInt52. (This should be handled by shouldSpeculateInt32 thing.
shouldSpeculateInt52 is now used to propagate Int52 format flow).

> Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:768
>		       break;

`shouldCheckOverflow(node->arithMode())` is always true here (this is also
asserted in DFGSpeculativeJIT.cpp).
We can remove this `|| !shouldCheckOverflow(node->arithMode()` part. By
dropping this part, the code looks well aligned to ArithAdd's one.

> Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:737
> +	       setPrediction(speculationFromValue(m_currentNode->asJSValue()));


More information about the webkit-reviews mailing list