We know that the JSC JIT and bmalloc both work on only a limited set of architectures. In Fedora, we have to manually disable these when building for s390x, ppc64, and ppc64le. But it's really easy to do the right thing automatically, so we might as well.
Created attachment 342881 [details] Patch
Comment on attachment 342881 [details] Patch Clearing flags on attachment: 342881 Committed r232918: <https://trac.webkit.org/changeset/232918>
All reviewed patches have been landed. Closing bug.
Nice one Michael!
*** Bug 175767 has been marked as a duplicate of this bug. ***
This is broken, the Debian ppc64el build (among others) fails now because it defaults to ENABLE_JIT=ON. See full build log here: https://buildd.debian.org/status/package.php?p=webkit2gtk&suite=experimental I tried to debug this a bit and I see that ENABLE_JIT_DEFAULT is set correctly (to OFF in this case) but later on the "Enabled features" list you can see that ENABLE_JIT is ON. With my zero CMake knowledge it seems that whatever ENABLE_JIT value is set in WebKitFeatures.cmake is later overridden in OptionsGTK.cmake by this line: WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_JIT PUBLIC ON) Setting it to ${ENABLE_JIT_DEFAULT} instead of ON appears to solve the problem, although I haven't run the full build yet. Also I notice that ENABLE_JIT is a PRIVATE option in WebKitFeatures.cmake but PUBLIC in OptionsGTK.cmake. Does that make sense?
(In reply to Alberto Garcia from comment #6) > With my zero CMake knowledge it seems that whatever ENABLE_JIT value > is set in WebKitFeatures.cmake is later overridden in OptionsGTK.cmake Same problem with USE_SYSTEM_MALLOC by the way.
(In reply to Alberto Garcia from comment #6) > WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_JIT PUBLIC ON) > > Setting it to ${ENABLE_JIT_DEFAULT} instead of ON appears to solve the > problem, although I haven't run the full build yet. I confirm that this fixes the build.
(In reply to Alberto Garcia from comment #6) > With my zero CMake knowledge it seems that whatever ENABLE_JIT value > is set in WebKitFeatures.cmake is later overridden in OptionsGTK.cmake > by this line: > > WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_JIT PUBLIC ON) Oops, yes indeed, good catch Berto. OptionsWPE.cmake has the same problem. Other ports are fine. I'm not really happy with depending on a variable that ought to be private to WebKitFeatures.cmake, so let me think about how to rework this check. > Setting it to ${ENABLE_JIT_DEFAULT} instead of ON appears to solve the > problem, although I haven't run the full build yet. > > Also I notice that ENABLE_JIT is a PRIVATE option in > WebKitFeatures.cmake but PUBLIC in OptionsGTK.cmake. Does that make > sense? Yes, all options are PRIVATE in WebKitFeatures.cmake to ensure ports control which options are exposed. Actually the only reason to have it in OptionsGTK.cmake is to change it to public.
(In reply to Michael Catanzaro from comment #9) > I'm not really happy with depending on a variable that ought to be private > to WebKitFeatures.cmake, so let me think about how to rework this check. I can't think of another way, so let's do this.
Actually no, let's just make the options private. They don't need to be public anymore as there is one right answer determined by architecture. Developers will know how to toggle them for testing regardless.
Created attachment 347672 [details] Patch
On a related note, the current code enables JIT by default on MIPS64, and that makes the build fail. Source/JavaScriptCore/assembler/MacroAssembler.h:69:2: error: #error "The MacroAssembler is not supported on this platform" I think this should be enough to fix the problem (I'm currently testing it): diff --git a/CMakeLists.txt b/CMakeLists.txt index 3fb3dd58a2b..062dcd02d09 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -87,6 +87,8 @@ if (LOWERCASE_CMAKE_SYSTEM_PROCESSOR MATCHES "^arm") set(WTF_CPU_ARM 1) elseif (LOWERCASE_CMAKE_SYSTEM_PROCESSOR MATCHES "^aarch64") set(WTF_CPU_ARM64 1) +elseif (LOWERCASE_CMAKE_SYSTEM_PROCESSOR MATCHES "^mips64") + set(WTF_CPU_MIPS64 1) elseif (LOWERCASE_CMAKE_SYSTEM_PROCESSOR MATCHES "^mips") set(WTF_CPU_MIPS 1) elseif (LOWERCASE_CMAKE_SYSTEM_PROCESSOR MATCHES "(x64|x86_64|amd64)")
(In reply to Alberto Garcia from comment #13) > On a related note, the current code enables JIT by default on MIPS64, and > that makes the build fail. > > Source/JavaScriptCore/assembler/MacroAssembler.h:69:2: error: #error "The > MacroAssembler is not supported on this platform" > > I think this should be enough to fix the problem (I'm currently testing it): r=me for this, but please land it in a separate bug.
Comment on attachment 347672 [details] Patch Attachment 347672 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8947672 New failing tests: http/tests/security/contentSecurityPolicy/video-with-https-url-allowed-by-csp-media-src-star.html
Created attachment 347834 [details] Archive of layout-test-results from ews206 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews206 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Ping any reviewers.
Related: bug 191941
*** Bug 191941 has been marked as a duplicate of this bug. ***
Created attachment 355563 [details] Patch
One problem with this approach is that the "Enabled features" list that cmake prints no longer shows whether the JIT, the sampling profiler or the system malloc are enabled or not.
(In reply to Alberto Garcia from comment #21) > One problem with this approach is that the "Enabled features" list that > cmake prints no longer shows whether the JIT, the sampling profiler or the > system malloc are enabled or not. That's desired, since WebKit now sets these and users no longer need to deal with them.
Fine with me then.
Comment on attachment 355563 [details] Patch Clearing flags on attachment: 355563 Committed r238619: <https://trac.webkit.org/changeset/238619>