Changeset 194707 in webkit

Timestamp:

Jan 7, 2016, 11:42:49 AM (9 years ago)

Author:

mark.lam@apple.com

Message:

ARMv7 or32(TrustedImm32, AbsoluteAddress) may have a bug with its use of dataTempRegister.
​https://bugs.webkit.org/show_bug.cgi?id=152833

Reviewed by Benjamin Poulain.

• assembler/MacroAssemblerARM.h:

(JSC::MacroAssemblerARM::or32):

• Added some assertions to make sure it is safe to use ARMRegisters::S0 as a temp.
• assembler/MacroAssemblerARM64.h:

(JSC::MacroAssemblerARM64::or32):

• Implement an optimization that avoids reloading the memoryTempRegister when the immediate is encodable as an instruction immediate.
• assembler/MacroAssemblerARMv7.h:

(JSC::MacroAssemblerARMv7::or32):

• Added an assertion to make sure it is safe to use the dataTempRegister as a temp.
• Implement an optimization that avoids reloading the memoryTempRegister when the immediate is encodable as an instruction immediate. In the event that we cannot encode the immediate, we'll use the addressTempRegister as a temp, and reload it later.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/MacroAssemblerARM.h (modified) (2 diffs)
• assembler/MacroAssemblerARM64.h (modified) (1 diff)
• assembler/MacroAssemblerARMv7.h (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-01-07  Mark Lam  <mark.lam@apple.com>
+
+        ARMv7 or32(TrustedImm32, AbsoluteAddress) may have a bug with its use of dataTempRegister.
+        https://bugs.webkit.org/show_bug.cgi?id=152833
+
+        Reviewed by Benjamin Poulain.
+
+        * assembler/MacroAssemblerARM.h:
+        (JSC::MacroAssemblerARM::or32):
+        - Added some assertions to make sure it is safe to use ARMRegisters::S0 as a temp.
+        * assembler/MacroAssemblerARM64.h:
+        (JSC::MacroAssemblerARM64::or32):
+        - Implement an optimization that avoids reloading the memoryTempRegister when
+          the immediate is encodable as an instruction immediate.
+        * assembler/MacroAssemblerARMv7.h:
+        (JSC::MacroAssemblerARMv7::or32):
+        - Added an assertion to make sure it is safe to use the dataTempRegister as a temp.
+        - Implement an optimization that avoids reloading the memoryTempRegister when
+          the immediate is encodable as an instruction immediate.  In the event that we
+          cannot encode the immediate, we'll use the addressTempRegister as a temp, and
+          reload it later.
+
 2016-01-07  Konstantin Tokarev  <annulen@yandex.ru>
 

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM.h

@@ -240 +240 @@
     void or32(TrustedImm32 imm, RegisterID dest)
     {
+        ASSERT(dest != ARMRegisters::S0);
         m_assembler.orrs(dest, dest, m_assembler.getImm(imm.m_value, ARMRegisters::S0));
     }
@@ -245 +246 @@
     void or32(TrustedImm32 imm, RegisterID src, RegisterID dest)
     {
+        ASSERT(src != ARMRegisters::S0);
         m_assembler.orrs(dest, src, m_assembler.getImm(imm.m_value, ARMRegisters::S0));
     }

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h

@@ -538 +538 @@
     void or32(TrustedImm32 imm, AbsoluteAddress address)
     {
-        load32(address.m_ptr, getCachedMemoryTempRegisterIDAndInvalidate());
-        or32(imm, memoryTempRegister, memoryTempRegister);
-        store32(memoryTempRegister, address.m_ptr);
+        LogicalImmediate logicalImm = LogicalImmediate::create32(imm.m_value);
+        if (logicalImm.isValid()) {
+            load32(address.m_ptr, getCachedDataTempRegisterIDAndInvalidate());
+            m_assembler.orr<32>(dataTempRegister, dataTempRegister, logicalImm);
+            store32(dataTempRegister, address.m_ptr);
+        } else {
+            load32(address.m_ptr, getCachedMemoryTempRegisterIDAndInvalidate());
+            or32(imm, memoryTempRegister, memoryTempRegister);
+            store32(memoryTempRegister, address.m_ptr);
+        }
     }
 

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h

@@ -355 +355 @@
     void or32(TrustedImm32 imm, AbsoluteAddress address)
     {
-        move(TrustedImmPtr(address.m_ptr), addressTempRegister);
-        load32(addressTempRegister, dataTempRegister);
-        or32(imm, dataTempRegister, dataTempRegister);
-        store32(dataTempRegister, addressTempRegister);
+        ARMThumbImmediate armImm = ARMThumbImmediate::makeEncodedImm(imm.m_value);
+        if (armImm.isValid()) {
+            move(TrustedImmPtr(address.m_ptr), addressTempRegister);
+            load32(addressTempRegister, dataTempRegister);
+            m_assembler.orr(dataTempRegister, dataTempRegister, armImm);
+            store32(dataTempRegister, addressTempRegister);
+        } else {
+            move(TrustedImmPtr(address.m_ptr), addressTempRegister);
+            load32(addressTempRegister, dataTempRegister);
+            move(imm, addressTempRegister);
+            m_assembler.orr(dataTempRegister, dataTempRegister, addressTempRegister);
+            move(TrustedImmPtr(address.m_ptr), addressTempRegister);
+            store32(dataTempRegister, addressTempRegister);
+        }
     }
 
@@ -384 +394 @@
             m_assembler.orr(dest, src, armImm);
         else {
+            ASSERT(src != dataTempRegister);
             move(imm, dataTempRegister);
             m_assembler.orr(dest, src, dataTempRegister);