I get Illegal instruction (core dumped) error message when JIT is enabled on IA-32 machines without SSE2. Reproduced on an Athlon XP. Also tested on Intel Celeron single core 2GHz (Northwood-128 architecture) which has SSE2 but on that it was OK, so the issue is SSE2.
Created attachment 346916 [details] Patch
Comment on attachment 346916 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346916&action=review > Source/cmake/OptionsGTK.cmake:-129 > -WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_JIT PUBLIC ON) Hm, I would rather keep this option PUBLIC. Can you change it to reuse ENABLE_JIT_DEFAULT, like this: WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_JIT PUBLIC ${ENABLE_JIT_DEFAULT}) That will be fragile, since it would be broken if the variable is removed from WebKitFeatures.cmake, but I don't see a better option, so also add a warning comment in WebKitFeatures.cmake to indicate the variable is surprisingly used in OptionsGTK.cmake. Then test that it actually works, of course. > Source/cmake/WebKitFeatures.cmake:75 > - if (WTF_CPU_ARM OR WTF_CPU_ARM64 OR WTF_CPU_MIPS OR WTF_CPU_X86_64 OR WTF_CPU_X86) > + if (WTF_CPU_ARM OR WTF_CPU_ARM64 OR WTF_CPU_MIPS OR WTF_CPU_X86_64 OR WTF_CPU_X86_SSE2) This is never set in CMake, so you're disabling it for everyone. Look in the toplevel CMakeLists.txt where WTF_CPU_X86 is set. You'll need to add some test for SSE2 instruction set there to ensure it gets defined when appropriate.
Created attachment 346962 [details] Patch
Why not disabling JIT at runtime at runtime/Options.cpp? Windows already does this. https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/Options.cpp#L400-L404
Created attachment 346968 [details] Patch
(In reply to Yusuke Suzuki from comment #4) > Why not disabling JIT at runtime at runtime/Options.cpp? > Windows already does this. > https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/ > Options.cpp#L400-L404 Thank you Yusuke, this is much simpler and more flexible, I tried it and it works on my Athlon XP.
Comment on attachment 346968 [details] Patch r=me
Comment on attachment 346968 [details] Patch Clearing flags on attachment: 346968 Committed r234784: <https://trac.webkit.org/changeset/234784>
All reviewed patches have been landed. Closing bug.
I think we need to remove OS(WINDOWS) from here: https://github.com/WebKit/webkit/blob/89aab857234c36c97bc0696c7d5bc03b475b6c6b/Source/JavaScriptCore/runtime/Options.cpp#L52 Maybe now it works only because something else is including MacroAssemblerX86.h but normally MacroAssembler.h would include it. Maybe the headers are included because of the source files are unified. This line would need MacroAssemblerX86.h to be included: MacroAssemblerX86::supportsFloatingPoint() What do you think?
Yes, please attach a follow-up patch.
Reopening
Created attachment 346984 [details] Patch
Comment on attachment 346984 [details] Patch Clearing flags on attachment: 346984 Committed r234787: <https://trac.webkit.org/changeset/234787>