The existing system makes implications from how many times a certain div slower path is taken. We can collect more accurate result profiling data.
Created attachment 268882 [details] work in progress for archiving.
Created attachment 268907 [details] rebased work in progress.
Created attachment 268939 [details] proposed fix.
Created attachment 268940 [details] x86_64 benchmark result.
From the attached x86_64 benchmark results, the following appear to be real (i.e. reproducible on retests): LongSpider crypto-aes 772.5283+-4.2715 ! 879.6557+-5.7639 ! definitely 1.1387x slower JSRegress adapt-to-double-divide 19.0715+-0.4259 ^ 17.5865+-0.2160 ^ definitely 1.0844x faster boolean-test 4.4102+-0.0386 ^ 3.9437+-0.0649 ^ definitely 1.1183x faster function-test 4.2060+-0.0576 ^ 3.6067+-0.0295 ^ definitely 1.1662x faster number-test 4.3185+-0.0423 ^ 3.9034+-0.0380 ^ definitely 1.1064x faster object-test 4.1588+-0.0930 ^ 3.6375+-0.1198 ^ definitely 1.1433x faster string-sub 53.1008+-1.3066 ^ 49.2135+-1.5713 ^ definitely 1.0790x faster string-test 4.2141+-0.0252 ^ 3.7333+-0.1327 ^ definitely 1.1288x faster undefined-test 4.4363+-0.0612 ^ 3.8700+-0.0653 ^ definitely 1.1463x faster Details on why crypto-aes is slower to follow after I gather a bit more data to make my case.
(In reply to comment #5) > From the attached x86_64 benchmark results, the following appear to be real > (i.e. reproducible on retests): > LongSpider > crypto-aes 772.5283+-4.2715 ! 879.6557+-5.7639 ! definitely 1.1387x slower > > Details on why crypto-aes is slower to follow after I gather a bit more data to make my case. The baseline reference build has 202 OSR exits. The patched build has 2322 OSR exits. Of those OSR exits, about 1179 is due to AESEncryptCtr#Dp5nqr, and 1153 is due to AESDecryptCtr#BpghTe. Note the count of OSR exits is imprecise due to concurrent JIT'ing. However, they are always in that ballpark. In contrast, with the baseline build, there were 202 OSR exits due to AESEncryptCtr#Dp5nqr, and 0 due to AESDecryptCtr#BpghTe. The AESEncryptCtr#Dp5nqr OSR exits are due to a pre-existing bug in how we use the Int52Rep node. See https://bugs.webkit.org/show_bug.cgi?id=153091. The AESDecryptCtr#BpghTe looks like it is due to the same issue manifested in a different way. Here's the IR: 435:<!3:loc17> ArithDiv(DoubleRep:@814<Double>, DoubleRep:@815<Double>, Double|MustGen|PureInt|MayHaveNonIntResult|UseAsInt, Int52, NotSet, Exits, bc#546) 436:<!0:-> MovHint(DoubleRep:@435<Double>, MustGen, loc24, W:SideState, ClobbersExit, bc#546, ExitInvalid) 438:< 1:loc18> JSConstant(JS|PureInt|UseAsInt, Boolint32, Int32: 1, bc#551) 817:< 1:loc20> Int52Rep(Check:DoubleRepMachineInt:@435<Double>, Int52|PureInt, Boolint32Nonboolint32Int52, Exits, bc#551) 818:< 1:loc19> Int52Constant(Int52|PureInt, Boolint32Nonboolint32Int52, Int32: 1, bc#551) 439:<!3:loc19> ArithSub(Int52Rep:@817<Int52>, Int52Rep:@818<Int52>, Int52|MustGen|PureInt|UseAsInt, Int52, CheckOverflow, Exits, bc#551) I haven't dug deep enough on this one yet, but note that there's also an Int52Rep node used to convert the result of an ArithDiv which produces a double. This is similar to the AESEncryptCtr#Dp5nqr case. I'm not sure what the right solution is yet: 1. Should the ArithDiv have coerced its result into an int because it was told that the result will be UseAsInt? 2. Should we have a ForceInt52Rep which will do truncation of fraction parts unlike the Int52Rep node?
The fix for https://bugs.webkit.org/show_bug.cgi?id=153379 has mitigated the regression due to OSR exits from AESDecryptCtr#BpghTe. However, I'm still seeing a significant increase in OSR exits from AESEncryptCtr#Dp5nqr. Continuing to investigate.
<rdar://problem/24685476>
The proposed fix for the OSR exit regression is https://bugs.webkit.org/show_bug.cgi?id=153091.