[Webkit-unassigned] [Bug 113862] New: Negative zero checks cause unnecessary speculation failures on SunSpider math-spectral-norm.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=113862

           Summary: Negative zero checks cause unnecessary speculation
                    failures on SunSpider math-spectral-norm.
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: Unspecified
        OS/Version: Unspecified
            Status: UNCONFIRMED
          Severity: Normal
          Priority: P2
         Component: JavaScriptCore
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: zhroma at ispras.ru


While testing SunSpider math-spectral-norm on x86-64 and ARMv7 Linux I found, that there are a lot of "Speculation failures" on ARM, but not on x86. All these speculation failures happen because of the following:

The hottest function in math-spectral-norm is A, it is inlined in some other functions on DFG. Function arguments are integer numbers.
function A(i,j) {
   return 1/((i+j)*(i+j+1)/2+i+1);
}

When DFG JIT creates code for the first division (by 2) -- it adds negative-zero checks. On x86 when ArithDiv divides integers x/y speculation failure happens only when (x == 0 && y < 0), and such check is inserted into assembly only when "!nodeCanIgnoreNegativeZero(node->arithNodeFlags())". But on ARM, FixupPhase thansforms division into 4 operations (convert x do double dx, convert y to double dy, division dz=dx/dy, convert dz to integer z if possible). And the last operation DoubleAsInt32 always makes a speculation failure when its integer result is zero. In the above function A every time when i and j are both zeros -- this check gives a speculation failure.

The main idea is that we should check nodeCanIgnoreNegativeZero() for DoubleAsInt32 nodes, and create zero-check only when needed.
Also, I found that NodeNeedsNegZero flag is wrongly set to 1 for this division. The result of this division is then added to constant (+i+1). The logic behind the code in BackwardPropagationPhase assumes that when one of ArithAdd operands is non-negative-zero constant we shouldn't check negative zero for the other operand. But the "isNotNegZero" function has wrong implementation: it actually checks for "isNegativeZero". Also in ArithSub node condition that allows to clear NodeNeedsNegZero flag can be extended.
Fixing all these issues allows us to have right NeedsNegZero flags and remove some unnecessary negative-zero checks. This gives a 22% speed up on the math-spectral-norm on ARMv7 Linux.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.