Bug 219288 – aarch64 llint does not build with JIT disabled

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Catanzaro]

Follow-up to bug #218613:

In file included from ../Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:30,
                 from ../Source/JavaScriptCore/assembler/MacroAssembler.h:46,
                 from ../Source/JavaScriptCore/jit/GPRInfo.h:28,
                 from ../Source/JavaScriptCore/bytecode/ArithProfile.h:28,
                 from ../Source/JavaScriptCore/llint/LLIntOffsetsExtractor.cpp:28:
../Source/JavaScriptCore/assembler/ARM64Assembler.h: In static member function 'static void JSC::ARM64Assembler::replaceWithJump(void*, void*)':
../Source/JavaScriptCore/assembler/ARM64Assembler.h:2576:51: error: 'class JSC::ExecutableAllocator' has no member named 'getJumpIslandTo'
             to = ExecutableAllocator::singleton().getJumpIslandTo(where, to);
                                                   ^~~~~~~~~~~~~~~
../Source/JavaScriptCore/assembler/ARM64Assembler.h: In static member function 'static void* JSC::ARM64Assembler::prepareForAtomicRelinkJumpConcurrently(void*, void*)':
../Source/JavaScriptCore/assembler/ARM64Assembler.h:2781:49: error: 'class JSC::ExecutableAllocator' has no member named 'getJumpIslandToConcurrently'
         return ExecutableAllocator::singleton().getJumpIslandToConcurrently(from, to);
                                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
../Source/JavaScriptCore/assembler/ARM64Assembler.h: In static member function 'static void JSC::ARM64Assembler::linkJumpOrCall(int*, const int*, void*)':
../Source/JavaScriptCore/assembler/ARM64Assembler.h:3024:51: error: 'class JSC::ExecutableAllocator' has no member named 'getJumpIslandTo'
             to = ExecutableAllocator::singleton().getJumpIslandTo(bitwise_cast<void*>(fromInstruction), to);
                                                   ^~~~~~~~~~~~~~~

I will turn off USE(JUMP_ISLANDS) and hope for the best.

[Michael Catanzaro]

CC Don since this kinda messes up the nice split between PlatformUse.h and PlatformEnable.h.

[Michael Catanzaro]

Created attachment 414869 [details]
Patch

[Michael Catanzaro]

The attached patch is for trunk.

It doesn't apply to webkit-2.30. This inline patch is needed for webkit-2.30:

diff --git a/Source/WTF/wtf/PlatformEnable.h b/Source/WTF/wtf/PlatformEnable.h
index 700f90adc6c3..f703d4239bc0 100644
--- a/Source/WTF/wtf/PlatformEnable.h
+++ b/Source/WTF/wtf/PlatformEnable.h
@@ -871,6 +871,6 @@
 #error "ENABLE(WEBGL2) requires ENABLE(WEBGL)"
 #endif
 
-#if CPU(ARM64) && CPU(ADDRESS64)
+#if CPU(ARM64) && CPU(ADDRESS64) && ENABLE(JIT)
 #define USE_JUMP_ISLANDS 1
 #endif

[Michael Catanzaro]

Comment on attachment 414869 [details]
Patch

Hm, there are even more problems, let me fix them all in one patch.

Next problem is:

../Source/JavaScriptCore/assembler/LinkBuffer.cpp: In member function 'void JSC::LinkBuffer::copyCompactAndLinkCode(JSC::MacroAssembler&, JSC::JITCompilationEffort)':
../Source/JavaScriptCore/assembler/LinkBuffer.cpp:380:13: error: there are no arguments to 'dumpJITMemory' that depend on a template parameter, so a declaration of 'dumpJITMemory' must be available [-fpermissive]
             dumpJITMemory(outData, outData, m_size);
             ^~~~~~~~~~~~~
../Source/JavaScriptCore/assembler/LinkBuffer.cpp:380:13: note: (if you use '-fpermissive', G++ will accept your code, but allowing the use of an undeclared name is deprecated)
../Source/JavaScriptCore/assembler/LinkBuffer.cpp: In instantiation of 'void JSC::LinkBuffer::copyCompactAndLinkCode(JSC::MacroAssembler&, JSC::JITCompilationEffort) [with InstructionType = unsigned int]':
../Source/JavaScriptCore/assembler/LinkBuffer.cpp:423:60:   required from here
../Source/JavaScriptCore/assembler/LinkBuffer.cpp:380:26: error: 'dumpJITMemory' was not declared in this scope
             dumpJITMemory(outData, outData, m_size);
             ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #3)
> The attached patch is for trunk.
> 
> It doesn't apply to webkit-2.30. This inline patch is needed for webkit-2.30:
> 
> diff --git a/Source/WTF/wtf/PlatformEnable.h
> b/Source/WTF/wtf/PlatformEnable.h
> index 700f90adc6c3..f703d4239bc0 100644
> --- a/Source/WTF/wtf/PlatformEnable.h
> +++ b/Source/WTF/wtf/PlatformEnable.h
> @@ -871,6 +871,6 @@
>  #error "ENABLE(WEBGL2) requires ENABLE(WEBGL)"
>  #endif
>  
> -#if CPU(ARM64) && CPU(ADDRESS64)
> +#if CPU(ARM64) && CPU(ADDRESS64) && ENABLE(JIT)
>  #define USE_JUMP_ISLANDS 1
>  #endif

Second patch for webkit-2.30:

diff --git a/Source/JavaScriptCore/assembler/LinkBuffer.cpp b/Source/JavaScriptCore/assembler/LinkBuffer.cpp
index b6577a9e8c4e..7745793670b1 100644
--- a/Source/JavaScriptCore/assembler/LinkBuffer.cpp
+++ b/Source/JavaScriptCore/assembler/LinkBuffer.cpp
@@ -374,6 +374,7 @@ void LinkBuffer::copyCompactAndLinkCode(MacroAssembler& macroAssembler, JITCompi
         m_executableMemory->shrink(m_size);
     }
 
+#if ENABLE(JIT)
     if (useFastJITPermissions()) {
         ASSERT(codeOutData == outData);
         if (UNLIKELY(Options::dumpJITMemoryPath()))
@@ -382,6 +383,10 @@ void LinkBuffer::copyCompactAndLinkCode(MacroAssembler& macroAssembler, JITCompi
         ASSERT(codeOutData != outData);
         performJITMemcpy(codeOutData, outData, m_size);
     }
+#else
+    ASSERT(codeOutData != outData);
+    performJITMemcpy(codeOutData, outData, m_size);
+#endif
 
     jumpsToLink.clear();

[Michael Catanzaro]

Created attachment 414873 [details]
Patch

[Michael Catanzaro]

(CC: Adrian for the webkit-2.30 portion of the patch, since the patch for trunk won't apply on webkit-2.30.)

[Radar WebKit Bug Importer]

<rdar://problem/71855960>

[Don Olmstead]

Comment on attachment 414873 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=414873&action=review

> Source/WTF/wtf/PlatformEnable.h:599
> +#define USE_JUMP_ISLANDS 1

Feels like maybe an #undef would be more appropriate?

[Michael Catanzaro]

Comment on attachment 414873 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=414873&action=review

>> Source/WTF/wtf/PlatformEnable.h:599
>> +#define USE_JUMP_ISLANDS 1
> 
> Feels like maybe an #undef would be more appropriate?

Uh... actually yes, good idea.

This might be the first time I've used #undef....

[Michael Catanzaro]

Created attachment 415154 [details]
Patch

[Michael Catanzaro]

Comment on attachment 415154 [details]
Patch

Sam Weinig suggests renaming this to ENABLE_JUMP_ISLANDS: "The intent of USE is “use a particular third-party library or optional OS service” (this is in a comment in top of the file)"

So I'll do that.

[Michael Catanzaro]

Created attachment 415237 [details]
Patch

[EWS]

Committed r270377: <https://trac.webkit.org/changeset/270377>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 415237 [details].