Modified: trunk/Source/_javascript_Core/ChangeLog (252727 => 252728)
--- trunk/Source/_javascript_Core/ChangeLog 2019-11-21 05:12:56 UTC (rev 252727)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-11-21 06:02:50 UTC (rev 252728)
@@ -1,3 +1,36 @@
+2019-11-20 Yusuke Suzuki <[email protected]>
+
+ [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):
+
2019-11-20 Saam Barati <[email protected]>
Baseline JIT should fill in StructureStubInfo's propertyIsInt32 and the slow path should update the array profile more frequently
Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h (252727 => 252728)
--- trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h 2019-11-21 05:12:56 UTC (rev 252727)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h 2019-11-21 06:02:50 UTC (rev 252728)
@@ -1254,8 +1254,7 @@
void load16(const void* address, RegisterID dest)
{
- moveToCachedReg(TrustedImmPtr(address), cachedMemoryTempRegister());
- m_assembler.ldrh(dest, memoryTempRegister, 0);
+ load<16>(address, dest);
}
void load16Unaligned(ImplicitAddress address, RegisterID dest)
@@ -1568,8 +1567,7 @@
void store16(RegisterID src, const void* address)
{
- moveToCachedReg(TrustedImmPtr(address), cachedMemoryTempRegister());
- m_assembler.strh(src, memoryTempRegister, 0);
+ store<16>(src, address);
}
void store16(TrustedImm32 imm, const void* address)
@@ -4220,12 +4218,12 @@
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;
}
}
@@ -4233,7 +4231,10 @@
if ((addressAsInt & (~maskHalfWord0)) == (currentRegisterContents & (~maskHalfWord0))) {
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;
}
}
@@ -4243,7 +4244,10 @@
cachedMemoryTempRegister().invalidate();
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);
}
template<int datasize>
@@ -4257,12 +4261,12 @@
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;
}
}
@@ -4270,7 +4274,10 @@
if ((addressAsInt & (~maskHalfWord0)) == (currentRegisterContents & (~maskHalfWord0))) {
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;
}
}
@@ -4277,7 +4284,10 @@
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);
}
template <int dataSize>
