Bug 219288

Summary: aarch64 llint does not build with JIT disabled
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: JavaScriptCoreAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, benjamin, cdumez, cmarcelo, darin, don.olmstead, ews-watchlist, fpizlo, guijemont, keith_miller, mark.lam, mcatanzaro, mgorse, msaboff, saam, sam, smoley, tzagallo, webkit-bug-importer, xan.lopez, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: PC   
OS: Linux   
See Also: https://bugs.webkit.org/show_bug.cgi?id=218613
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Michael Catanzaro 2020-11-25 14:36:24 PST
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.
Comment 1 Michael Catanzaro 2020-11-25 14:41:05 PST
CC Don since this kinda messes up the nice split between PlatformUse.h and PlatformEnable.h.
Comment 2 Michael Catanzaro 2020-11-25 14:41:17 PST
Created attachment 414869 [details]
Patch
Comment 3 Michael Catanzaro 2020-11-25 14:55:14 PST
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
Comment 4 Michael Catanzaro 2020-11-25 15:13:44 PST
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);
             ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
Comment 5 Michael Catanzaro 2020-11-25 17:53:36 PST
(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();
Comment 6 Michael Catanzaro 2020-11-25 17:55:36 PST
Created attachment 414873 [details]
Patch
Comment 7 Michael Catanzaro 2020-11-25 17:59:26 PST
(CC: Adrian for the webkit-2.30 portion of the patch, since the patch for trunk won't apply on webkit-2.30.)
Comment 8 Radar WebKit Bug Importer 2020-12-01 10:18:36 PST
<rdar://problem/71855960>
Comment 9 Don Olmstead 2020-12-01 11:13:59 PST
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?
Comment 10 Michael Catanzaro 2020-12-01 12:19:14 PST
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....
Comment 11 Michael Catanzaro 2020-12-01 12:30:39 PST
Created attachment 415154 [details]
Patch
Comment 12 Michael Catanzaro 2020-12-02 09:37:48 PST
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.
Comment 13 Michael Catanzaro 2020-12-02 11:07:40 PST
Created attachment 415237 [details]
Patch
Comment 14 EWS 2020-12-02 16:43:24 PST
Committed r270377: <https://trac.webkit.org/changeset/270377>

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