[JSC] Extend MacroAssemblerARM64::load/store for datasize = 16
Created attachment 384026 [details] Patch WIP
<rdar://problem/57366761>
Created attachment 384027 [details] Patch WIP
Created attachment 384029 [details] Patch
Comment on attachment 384029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384029&action=review > Source/JavaScriptCore/ChangeLog:9 > + Our `void load16(const void* address, RegisterID dest)` and `void store16(RegisterID src, const void* address)` do not aware of /do not aware/are not aware/. > Source/JavaScriptCore/ChangeLog:10 > + the condition that passed register is memoryTempRegister, while `MacroAssemblerARM64::{load,store}` handles it correctly, e.g. /is memoryTempRegister/can be memoryTempRegister/ > Source/JavaScriptCore/ChangeLog:24 > + to support 16 so that `or16` implementation looks like almost the same to `or32` etc. /looks like almost the same to/is similar to/ > Source/JavaScriptCore/assembler/testmasm.cpp:1137 > + // Just compiling test. > + compile([&] (CCallHelpers& jit) { > + emitFunctionPrologue(jit); > + jit.or16(CCallHelpers::TrustedImm32(42), CCallHelpers::AbsoluteAddress(nullptr)); > + emitFunctionEpilogue(jit); > + jit.ret(); > + }); How is this test case different than the one immediately above it?
Comment on attachment 384029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384029&action=review >> Source/JavaScriptCore/assembler/testmasm.cpp:1137 >> + }); > > How is this test case different than the one immediately above it? nullptr is always invalid logical imm in ARM64, so this takes the path this patch is fixing.
Comment on attachment 384029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384029&action=review r=me with fixes. >>> Source/JavaScriptCore/assembler/testmasm.cpp:1137 >>> + }); >> >> How is this test case different than the one immediately above it? > > nullptr is always invalid logical imm in ARM64, so this takes the path this patch is fixing. Yusuke told me offline that his intent is to take the !logicalImm.isValid() case in or16. For that, we need to pass TrustedImm(0), not AbsoluteAddress(nullptr).
Comment on attachment 384029 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384029&action=review Thanks! >> Source/JavaScriptCore/ChangeLog:9 >> + Our `void load16(const void* address, RegisterID dest)` and `void store16(RegisterID src, const void* address)` do not aware of > > /do not aware/are not aware/. Fixed. >> Source/JavaScriptCore/ChangeLog:10 >> + the condition that passed register is memoryTempRegister, while `MacroAssemblerARM64::{load,store}` handles it correctly, e.g. > > /is memoryTempRegister/can be memoryTempRegister/ Fixed. >> Source/JavaScriptCore/ChangeLog:24 >> + to support 16 so that `or16` implementation looks like almost the same to `or32` etc. > > /looks like almost the same to/is similar to/ Fixed. >>>> Source/JavaScriptCore/assembler/testmasm.cpp:1137 >>>> + }); >>> >>> How is this test case different than the one immediately above it? >> >> nullptr is always invalid logical imm in ARM64, so this takes the path this patch is fixing. > > Yusuke told me offline that his intent is to take the !logicalImm.isValid() case in or16. For that, we need to pass TrustedImm(0), not AbsoluteAddress(nullptr). Nice, fixed.
Committed r252728: <https://trac.webkit.org/changeset/252728>