Changeset 196591 in webkit


Ignore:
Timestamp:
Feb 15, 2016 1:08:47 PM (8 years ago)
Author:
mark.lam@apple.com
Message:

[ARMv7] stress/op_rshift.js and stress/op_urshift.js are failing.
https://bugs.webkit.org/show_bug.cgi?id=151514

Reviewed by Filip Pizlo.

The issue turns out to be trivial: on ARMv7 (and traditional ARM too), arithmetic
shift right (ASR) and logical shift right (LSR) takes an immediate shift amount
from 1-32. See http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204j/Cjacbgca.html.
An immediate shift amount of 0 is interpreted as a shift of 32 bits.

Meanwhile, our macro assembler is expecting the immediate shift value to be
between 0-31. As a result, a shift amount of 0 is being wrongly encoded with 0
bits which means shift right by 32 bits.

The fix is to check if the shift amount is 0, and if so, emit a move. Else,
emit the right shift as usual.

This issue does not affect left shifts, as the immediate shift amount for left
shifts is between 0-31 as our macro assembler expects.

  • assembler/MacroAssemblerARM.h:

(JSC::MacroAssemblerARM::rshift32):
(JSC::MacroAssemblerARM::urshift32):
(JSC::MacroAssemblerARM::sub32):

  • assembler/MacroAssemblerARMv7.h:

(JSC::MacroAssemblerARMv7::rshift32):
(JSC::MacroAssemblerARMv7::urshift32):

  • tests/stress/op_rshift.js:
  • tests/stress/op_urshift.js:
  • Un-skip these tests. They should always pass now.
Location:
trunk/Source/JavaScriptCore
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r196587 r196591  
     12016-02-15  Mark Lam  <mark.lam@apple.com>
     2
     3        [ARMv7] stress/op_rshift.js and stress/op_urshift.js are failing.
     4        https://bugs.webkit.org/show_bug.cgi?id=151514
     5
     6        Reviewed by Filip Pizlo.
     7
     8        The issue turns out to be trivial: on ARMv7 (and traditional ARM too), arithmetic
     9        shift right (ASR) and logical shift right (LSR) takes an immediate shift amount
     10        from 1-32.  See http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0204j/Cjacbgca.html.
     11        An immediate shift amount of 0 is interpreted as a shift of 32 bits.
     12
     13        Meanwhile, our macro assembler is expecting the immediate shift value to be
     14        between 0-31.  As a result, a shift amount of 0 is being wrongly encoded with 0
     15        bits which means shift right by 32 bits.
     16
     17        The fix is to check if the shift amount is 0, and if so, emit a move.  Else,
     18        emit the right shift as usual.
     19
     20        This issue does not affect left shifts, as the immediate shift amount for left
     21        shifts is between 0-31 as our macro assembler expects.
     22
     23        * assembler/MacroAssemblerARM.h:
     24        (JSC::MacroAssemblerARM::rshift32):
     25        (JSC::MacroAssemblerARM::urshift32):
     26        (JSC::MacroAssemblerARM::sub32):
     27        * assembler/MacroAssemblerARMv7.h:
     28        (JSC::MacroAssemblerARMv7::rshift32):
     29        (JSC::MacroAssemblerARMv7::urshift32):
     30
     31        * tests/stress/op_rshift.js:
     32        * tests/stress/op_urshift.js:
     33        - Un-skip these tests.  They should always pass now.
     34
    1352016-02-15  Filip Pizlo  <fpizlo@apple.com>
    236
  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM.h

    r194707 r196591  
    275275    void rshift32(RegisterID src, TrustedImm32 imm, RegisterID dest)
    276276    {
    277         m_assembler.movs(dest, m_assembler.asr(src, imm.m_value & 0x1f));
     277        if (!imm.m_value)
     278            move(src, dest);
     279        else
     280            m_assembler.movs(dest, m_assembler.asr(src, imm.m_value & 0x1f));
    278281    }
    279282
     
    298301    void urshift32(RegisterID src, TrustedImm32 imm, RegisterID dest)
    299302    {
    300         m_assembler.movs(dest, m_assembler.lsr(src, imm.m_value & 0x1f));
     303        if (!imm.m_value)
     304            move(src, dest);
     305        else
     306            m_assembler.movs(dest, m_assembler.lsr(src, imm.m_value & 0x1f));
    301307    }
    302308
  • trunk/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h

    r196152 r196591  
    401401    void rshift32(RegisterID src, TrustedImm32 imm, RegisterID dest)
    402402    {
    403         m_assembler.asr(dest, src, imm.m_value & 0x1f);
     403        if (!imm.m_value)
     404            move(src, dest);
     405        else
     406            m_assembler.asr(dest, src, imm.m_value & 0x1f);
    404407    }
    405408
     
    426429    void urshift32(RegisterID src, TrustedImm32 imm, RegisterID dest)
    427430    {
    428         m_assembler.lsr(dest, src, imm.m_value & 0x1f);
     431        if (!imm.m_value)
     432            move(src, dest);
     433        else
     434            m_assembler.lsr(dest, src, imm.m_value & 0x1f);
    429435    }
    430436
  • trunk/Source/JavaScriptCore/tests/stress/op_rshift.js

    r193788 r196591  
    1 //@ skip
    2 // FIXME: https://bugs.webkit.org/show_bug.cgi?id=151514
     1//@ runFTLNoCJIT
    32
    43// If all goes well, this test module will terminate silently. If not, it will print
  • trunk/Source/JavaScriptCore/tests/stress/op_urshift.js

    r193788 r196591  
    1 //@ skip
    2 // FIXME: https://bugs.webkit.org/show_bug.cgi?id=151514
     1//@ runFTLNoCJIT
    32
    43// If all goes well, this test module will terminate silently. If not, it will print
Note: See TracChangeset for help on using the changeset viewer.