[Webkit-unassigned] [Bug 191548] Enable JIT on ARM/Linux

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=191548

Yusuke Suzuki <yusukesuzuki at slowstart.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #355322|review?, commit-queue?      |review+, commit-queue-
              Flags|                            |

--- Comment #5 from Yusuke Suzuki <yusukesuzuki at slowstart.org> ---
Comment on attachment 355322
  --> https://bugs.webkit.org/attachment.cgi?id=355322
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?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20181120/fc9fc0c5/attachment.html>