Bug 67486 - MacroAssemberARM branchTruncateDoubleToInt32() doesn't work?
Summary: MacroAssemberARM branchTruncateDoubleToInt32() doesn't work?
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2011-09-02 07:20 PDT by Yong Li
Modified: 2011-09-06 19:00 PDT (History)
3 users (show)

See Also:

the patch that turns off this optimization (4.94 KB, patch)
2011-09-06 12:01 PDT, Yong Li
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yong Li 2011-09-02 07:20:58 PDT
Right shift operators (>> and >>>) fail when the left operand is a large negative number and the right operand is an immediate number.

var largeNeg=-2715228072;
alert(largeNeg >>> 5); // wrong when using ARM assembler.

It seems branchTruncateDoubleToInt32() doesn't work as expected by the caller (JIT::emitRightShiftSlowCase).

Turning off supportsFloatingPointTruncate() for MacroAssemberARM can simply solve the problem.

I noticed this comment in MacroAssemberARMv7.h:

    // On x86(_64) the MacroAssembler provides an interface to truncate a double to an integer.
    // If a value is not representable as an integer, and possibly for some values that are,
    // (on x86 INT_MIN, since this is indistinguishable from results for out-of-range/NaN input)
    // a branch will  be taken.  It is not clear whether this interface will be well suited to
    // other platforms.  On ARMv7 the hardware truncation operation produces multiple possible
    // failure values (saturates to INT_MIN & INT_MAX, NaN reulsts in a value of 0).  This is a
    // temporary solution while we work out what this interface should be.  Either we need to
    // decide to make this interface work on all platforms, rework the interface to make it more
    // generic, or decide that the MacroAssembler cannot practically be used to abstracted these
    // operations, and make clients go directly to the m_assembler to plant truncation instructions.
    // In short, FIXME:.
    bool supportsFloatingPointTruncate() const { return false; }

Should we also turn off supportsFloatingPointTruncate() for ARM?
Comment 1 Yong Li 2011-09-06 12:01:12 PDT
Created attachment 106457 [details]
the patch that turns off this optimization
Comment 2 WebKit Review Bot 2011-09-06 19:00:22 PDT
Comment on attachment 106457 [details]
the patch that turns off this optimization

Clearing flags on attachment: 106457

Committed r94622: <http://trac.webkit.org/changeset/94622>
Comment 3 WebKit Review Bot 2011-09-06 19:00:27 PDT
All reviewed patches have been landed.  Closing bug.