Bug 153091 - DFG's Int52Rep node is inadequate to convert doubles to Int52.
Summary: DFG's Int52Rep node is inadequate to convert doubles to Int52.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks: 153019
  Show dependency treegraph
 
Reported: 2016-01-13 22:47 PST by Mark Lam
Modified: 2016-05-31 17:20 PDT (History)
7 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2016-01-13 22:47:48 PST
With the patch for https://bugs.webkit.org/show_bug.cgi?id=153019, we can see the following IR for a function AESEncryptCtr#Dp5nqr:

 483:<!3:loc24>	ArithDiv(DoubleRep:@1009<Double>, DoubleRep:@1010<Double>, Double|MustGen|PureInt|MayHaveNonIntResult|UseAsInt, Int52, NotSet, Exits, bc#598)
 484:<!0:->	MovHint(DoubleRep:@483<Double>, MustGen, loc26, W:SideState, ClobbersExit, bc#598, ExitInvalid)
 486:< 1:loc28>	JSConstant(JS|UseAsInt, Nonboolint32, Int32: 8, bc#603)
 487:<!2:loc28>	ArithMul(Int32:@473, Int32:@486, Number|MustGen|PureInt|UseAsInt, Int32, Unchecked, Exits, bc#603)
 488:<!0:->	MovHint(Untyped:@487, MustGen, loc27, W:SideState, ClobbersExit, bc#603, ExitInvalid)
 1012:< 1:loc26>	Int52Rep(Check:DoubleRepMachineInt:@483<Double>, Int52|PureInt, Boolint32Nonboolint32Int52, Exits, bc#608)
 983:< 1:loc26>	ValueToInt32(Int52Rep:@1012<Int52>, Int32|PureInt, Int32, Exits, bc#608)
 490:< 2:loc28>	BitURShift(KnownInt32:@983, Int32:@487, Int32|UseAsOther, Int32, Exits, bc#608)
 1289:<!0:->	Phantom(Check:DoubleRep:@483<Double>, MustGen, bc#608)
 491:<!0:->	MovHint(Untyped:@490, MustGen, loc26, W:SideState, ClobbersExit, bc#608, ExitInvalid)
 493:< 2:loc28>	UInt32ToNumber(Int32:@490, Number|UseAsOther, Int32, CheckOverflow, Exits, bc#612)
  
AESEncryptCtr#Dp5nqr is OSR exiting at the Int52Rep node (@1012) because it fails to convert the double result of the div (@483) to an Int52.  The double value is between 0 and 1.  The value would have been used by the BitURShift later.  The implementation expects to just truncate the fraction and convert that number to an int 0.

However, the implementation of DFG Int52Rep for converting doubles calls tryConvertToInt52() which will only convert the double to an Int52 if and only if the value it contains is already an Int52 value expressed in double form.  It will not truncate the fraction part as we expect.

As a result, the Int52Rep node will trigger an OSR exit (which we don't want).
Comment 1 Radar WebKit Bug Importer 2016-01-14 12:06:18 PST
<rdar://problem/24192607>
Comment 2 Benjamin Poulain 2016-05-31 17:20:47 PDT
I suspect the real problem was that PredictionPropagation was setting absurd values after https://bugs.webkit.org/show_bug.cgi?id=153019.

I reverted that change then Filip fixed the whole type prediction some time later. We no longer speculate Int32 for a ArithDiv that produce double.

I tried crypto-aes.js on my machine. The only OSR Exit is a legit Overflow on ArithDiv due to a rare int/int producing double.
The Div at bc#598 is now producing a double as it should.