[webkit-reviews] review requested: [Bug 113862] Negative zero checks cause unnecessary speculation failures on SunSpider on ARMv7 : [Attachment 199746] patch with smaller layout tests

[bugzilla-daemon at webkit.org]

Roman Zhuykov <zhroma at ispras.ru> has asked  for review:
Bug 113862: Negative zero checks cause unnecessary speculation failures on
SunSpider on ARMv7
https://bugs.webkit.org/show_bug.cgi?id=113862

Attachment 199746: patch with smaller layout tests
https://bugs.webkit.org/attachment.cgi?id=199746&action=review

------- Additional Comments from Roman Zhuykov <zhroma at ispras.ru>
(In reply to comment #9)
> (From update of attachment 197263 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=197263&action=review
> 
> r=me
> 
> cq- because of the layout test issue. Any committer can fix the layout test
and then land, or you can post a new patch, and I can cq+ it.
> 
> > LayoutTests/fast/js/regress/script-tests/negative-zero-divide.js:6
> > +  for (i = 0; i < 100000; i++) {
> 
> Please change this to 100. That's our de facto limit for getting code to run
in the DFG, without making regression tests run too long.

Thank you for your review. Unfortunately, "i < 100" is not enough. In
script-tests directory there are 95 tests, 71 of them have loops with more than
10000 iterations, 55 tests have more than 100000 loop iterations. I've just
tested my negative-zero-negate.js on ARMv7 Linux. The test starts to fail
(without my patch) only with "i < 1094" or more loop iterations. Checking with
DFG_ENABLE_DEBUG_VERBOSE=1 shows, that there are no DFG compilations when I use
"i < 1093" or smaller values. I have created the new patch with "i < 2001".
This is fast enough, and give some reserve if DFG-starting parameters would
change later. Is patch now OK?