Changeset 252728 in webkit for trunk/Source/JavaScriptCore/assembler

Timestamp:

Nov 20, 2019, 10:02:50 PM (6 years ago)

Author:

[email protected]

Message:

[JSC] Extend MacroAssemblerARM64::load/store for datasize = 16
​https://bugs.webkit.org/show_bug.cgi?id=204442
<rdar://problem/57366761>

Reviewed by Mark Lam.

Our void load16(const void* address, RegisterID dest) and void store16(RegisterID src, const void* address) are not aware of
the condition that passed register can be memoryTempRegister, while MacroAssemblerARM64::{load,store} handles it correctly, e.g.
load invalidates cachedMemoryTempRegister if destination register is memoryTempRegister. As a result, when we are emitting
or16(TrustedImm32 imm, AbsoluteAddress address) with address where the address's value does not fit in imm, the generated code
is reusing memoryTempRegister incorrectly.

0xedf8d4fb4: mov x17, #0x7af0
0xedf8d4fb8: movk x17, #0xd5a, lsl #16
0xedf8d4fbc: movk x17, #0x1, lsl #32 Construct imm register on x17.
0xedf8d4fc0: ldrh w17, [x17] Load half word from x17 to w17 (we should invalidate x17 memoryTempRegister here).
0xedf8d4fc4: mov w16, #0x1b
0xedf8d4fc8: orr w16, w17, w16
0xedf8d4fcc: strh w16, [x17] x17 memoryTempRegister is reused while its content is invalid.

The problem is that load and store functions are not supporting datasize = 16 case. This patch extends MacroAssemblerARM64::{load,store}
to support 16 so that or16 implementation looks is similar to or32 etc.

• assembler/MacroAssemblerARM64.h:

(JSC::MacroAssemblerARM64::load16):
(JSC::MacroAssemblerARM64::store16):
(JSC::MacroAssemblerARM64::load):
(JSC::MacroAssemblerARM64::store):

• assembler/testmasm.cpp:

(JSC::testOrImmMem):

Location:

trunk/Source/JavaScriptCore/assembler

Files:

• MacroAssemblerARM64.h (modified) (8 diffs)
• testmasm.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h

@@ -1255 +1255 @@
     void load16(const void* address, RegisterID dest)
     {
-        moveToCachedReg(TrustedImmPtr(address), cachedMemoryTempRegister());
-        m_assembler.ldrh(dest, memoryTempRegister, 0);
+        load<16>(address, dest);
     }
 
@@ -1569 +1568 @@
     void store16(RegisterID src, const void* address)
     {
-        moveToCachedReg(TrustedImmPtr(address), cachedMemoryTempRegister());
-        m_assembler.strh(src, memoryTempRegister, 0);
+        store<16>(src, address);
     }
 
@@ -4221 +4219 @@
             if (isInt<32>(addressDelta)) {
                 if (Assembler::canEncodeSImmOffset(addressDelta)) {
-                    m_assembler.ldur<datasize>(dest,  memoryTempRegister, addressDelta);
+                    loadUnscaledImmediate<datasize>(dest, memoryTempRegister, addressDelta);
                     return;
                 }
 
                 if (Assembler::canEncodePImmOffset<datasize>(addressDelta)) {
-                    m_assembler.ldr<datasize>(dest,  memoryTempRegister, addressDelta);
+                    loadUnsignedImmediate<datasize>(dest, memoryTempRegister, addressDelta);
                     return;
                 }
@@ -4234 +4232 @@
                 m_assembler.movk<64>(memoryTempRegister, addressAsInt & maskHalfWord0, 0);
                 cachedMemoryTempRegister().setValue(reinterpret_cast<intptr_t>(address));
-                m_assembler.ldr<datasize>(dest, memoryTempRegister, ARM64Registers::zr);
+                if constexpr (datasize == 16)
+                    m_assembler.ldrh(dest, memoryTempRegister, ARM64Registers::zr);
+                else
+                    m_assembler.ldr<datasize>(dest, memoryTempRegister, ARM64Registers::zr);
                 return;
             }
@@ -4244 +4245 @@
         else
             cachedMemoryTempRegister().setValue(reinterpret_cast<intptr_t>(address));
-        m_assembler.ldr<datasize>(dest, memoryTempRegister, ARM64Registers::zr);
+        if constexpr (datasize == 16)
+            m_assembler.ldrh(dest, memoryTempRegister, ARM64Registers::zr);
+        else
+            m_assembler.ldr<datasize>(dest, memoryTempRegister, ARM64Registers::zr);
     }
 
@@ -4258 +4262 @@
             if (isInt<32>(addressDelta)) {
                 if (Assembler::canEncodeSImmOffset(addressDelta)) {
-                    m_assembler.stur<datasize>(src, memoryTempRegister, addressDelta);
+                    storeUnscaledImmediate<datasize>(src, memoryTempRegister, addressDelta);
                     return;
                 }
 
                 if (Assembler::canEncodePImmOffset<datasize>(addressDelta)) {
-                    m_assembler.str<datasize>(src, memoryTempRegister, addressDelta);
+                    storeUnsignedImmediate<datasize>(src, memoryTempRegister, addressDelta);
                     return;
                 }
@@ -4271 +4275 @@
                 m_assembler.movk<64>(memoryTempRegister, addressAsInt & maskHalfWord0, 0);
                 cachedMemoryTempRegister().setValue(reinterpret_cast<intptr_t>(address));
-                m_assembler.str<datasize>(src, memoryTempRegister, ARM64Registers::zr);
+                if constexpr (datasize == 16)
+                    m_assembler.strh(src, memoryTempRegister, ARM64Registers::zr);
+                else
+                    m_assembler.str<datasize>(src, memoryTempRegister, ARM64Registers::zr);
                 return;
             }
@@ -4278 +4285 @@
         move(TrustedImmPtr(address), memoryTempRegister);
         cachedMemoryTempRegister().setValue(reinterpret_cast<intptr_t>(address));
-        m_assembler.str<datasize>(src, memoryTempRegister, ARM64Registers::zr);
+        if constexpr (datasize == 16)
+            m_assembler.strh(src, memoryTempRegister, ARM64Registers::zr);
+        else
+            m_assembler.str<datasize>(src, memoryTempRegister, ARM64Registers::zr);
     }
 

trunk/Source/JavaScriptCore/assembler/testmasm.cpp

@@ -1128 +1128 @@
     invoke<void>(or16);
     CHECK_EQ(memoryLocation, 0x12341234 | 42);
+
+    memoryLocation = 0x12341234;
+    auto or16InvalidLogicalImmInARM64 = compile([&] (CCallHelpers& jit) {
+        emitFunctionPrologue(jit);
+        jit.or16(CCallHelpers::TrustedImm32(0), CCallHelpers::AbsoluteAddress(&memoryLocation));
+        emitFunctionEpilogue(jit);
+        jit.ret();
+    });
+    invoke<void>(or16InvalidLogicalImmInARM64);
+    CHECK_EQ(memoryLocation, 0x12341234);
 }