Enable JIT on ARM/Linux
Created attachment 354562 [details] Patch
This patch enables the baseline JIT on ARM/Linux. It also contains patch https://bugs.webkit.org/show_bug.cgi?id=191256, since that hasn't been reviewed yet. It probably makes sense to land them separately.
Could you rebase the patch?
Created attachment 355322 [details] Patch
Comment on attachment 355322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=355322&action=review r=me with comments. > Source/JavaScriptCore/jit/JITCall32_64.cpp:298 > + if (compileCallEval(bytecode)) { This brace is not necessary. > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:628 > +elsif ARMv7 > + macro probe(action) > + push a0 > + push a1 > + push a2 > + push a3 > + push t0 > + push t1 > + push t2 > + push t3 > + push t4 > + push t5 > + push lr > + push csr0 > + > + action() > + > + pop csr0 > + pop lr > + pop t5 > + pop t4 > + pop t3 > + pop t2 > + pop t1 > + pop t0 > + pop a3 > + pop a2 > + pop a1 > + pop a0 > + end The implementation is almost the same to the above implementation for X86_64 or ARM64 or ARM64E. Can we share the code with it? > Source/JavaScriptCore/runtime/SamplingProfiler.cpp:447 > + > + if (!codeBlock->instructions().size()) { > + isValid = false; > + return 0; > + } I think this is unnecessary if we add InstructionStream::contains, which is described below. > Source/JavaScriptCore/runtime/SamplingProfiler.cpp:451 > + const Instruction* begin = codeBlock->instructions().at(0).ptr(); > + const Instruction* end = reinterpret_cast<const Instruction*>(reinterpret_cast<const uint8_t*>(begin) + codeBlock->instructions().size()); > + if (instruction >= begin && instruction < end) { I think this should be encapsulated in InstructionStream class. Adding InstructionStream::contains, and if (codeBlock->instructions().contains(nstruction)) { looks better. > Source/cmake/WebKitFeatures.cmake:71 > elseif (WTF_CPU_ARM AND WTF_OS_UNIX) Should we switch this `UNIX` to `LINUX` because `OS(LINUX)` is checked in Platform.h?
Created attachment 355356 [details] Patch
Thanks for the review! I think I've addressed your comments. The braces are removed. The probe-macro from x64 and arm64 is now also used for arm. I added InstructionStream::contains and in CMakeLists.txt I am now using WTF_OS_LINUX instead of WTF_OS_UNIX (I had to introduce that flag).
Comment on attachment 355356 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=355356&action=review r=me > Source/JavaScriptCore/bytecode/InstructionStream.cpp:49 > + const Instruction* begin = at(0).ptr(); > + const Instruction* end = reinterpret_cast<const Instruction*>(reinterpret_cast<const uint8_t*>(begin) + size()); > + > + return instruction >= begin && instruction < end; Actually, you do not need to use `Instruction*` here. You can define this function simply by, const uint8_t* pointer = bitwise_cast<const uint8_t*>(instruction); return pointer >= m_instructions.data() && pointer < (m_instructions.data() + m_instructions.size()); > Source/JavaScriptCore/jit/GPRInfo.h:531 > +#define NUMBER_OF_CALLEE_SAVES_REGISTERS 1u This is only correct for ARM_THUMB2.
Created attachment 355395 [details] Patch
Comment on attachment 355395 [details] Patch r=me, nice!
Comment on attachment 355395 [details] Patch Clearing flags on attachment: 355395 Committed r238414: <https://trac.webkit.org/changeset/238414>
All reviewed patches have been landed. Closing bug.
<rdar://problem/46198694>