* ENABLE_LLINT_C_LOOP is the opposite of ENABLE_JIT. It would be nonsense to have both or neither enabled, so there's no need for these to be separate options. * Add ENABLE_DFG_JIT and have ENABLE_FTL_JIT depend on it. * Add ENABLE_FTL_NATIVE_CALL_INLINING option, dependent on ENABLE_FTL_JIT. If you currently build with -DENABLE_LLINT_C_LOOP=ON, you will instead want to build with -DENABLE_JIT=OFF. If you currently build with -DENABLE_JIT=ON on a platform that supports baseline JIT but not DFG JIT, you'll need to add -DENABLE_DFG_JIT=OFF.
This makes a lot of sense!
Note, this patch in its current form is part of a long chain of patches (hence no r? yet), and needs to be committed alongside the patch in bug #143997 (or changed to not depend on that). Also note I don't touch the Xcode files here in an attempt to not break anything on Macs, only the CMake files, since they did not already have an ENABLE_JIT option. That would be a good follow-up. I did remove the option from FeatureList.pm, since I think that won't break anything. Alternative: remove the ENABLE_JIT option from the CMake port, and keep just ENABLE_LLINT_C_LOOP. I thought ENABLE_JIT was a nicer option to present to users, but the Xcode files and FeatureList.pm have ENABLE_LLINT_C_LOOP but not ENABLE_JIT, so this would make things consistent between both ports without requiring changes anywhere but the CMake files.
Created attachment 251242 [details] Patch
I took the time to do this properly, so it no longer depends on bug #143997.
Created attachment 251248 [details] Patch
Comment on attachment 251248 [details] Patch Ah but it still depends on bug #143831, whoops....
Created attachment 251359 [details] Patch
Comment on attachment 251359 [details] Patch Clearing flags on attachment: 251359 Committed r183159: <http://trac.webkit.org/changeset/183159>
All reviewed patches have been landed. Closing bug.
Comment on attachment 251359 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=251359&action=review > Tools/Scripts/webkitperl/FeatureList.pm:-438 > - { option => "cloop", desc => "Force use of the llint c loop", > - define => "ENABLE_LLINT_C_LOOP", default => 0, value => \$forceCLoop }, With this change we lost build-webkit's --cloop option. Of course, --cmakeargs="-DENABLE_JIT=0" still works. (I haven't used --cloop option of build-webkit ever and don't plan to do it in the future, just noticed it.)
Ah, I didn't realize that list of options was used for anything besides the default values. I think we should probably bring that back, as otherwise you won't be able to use build-webkit on various architectures. Options: (a) convert the XCode build to use ENABLE_JIT so that we can restore the option to FeatureList.pm, or (b) revert this patch and instead remove the ENABLE_JIT option from the CMake build. Maybe (b) is the way to go. When I wrote this patch, I did not realize that only CMake had the ENABLE_JIT option.
Created attachment 251439 [details] [CMake] Should be possible for an option to depend on multiple options
(Ignore that bogus attachment.) (In reply to comment #11) > Options: (a) convert the XCode build to use ENABLE_JIT so that we can > restore the option to FeatureList.pm, or (b) revert this patch and instead > remove the ENABLE_JIT option from the CMake build. > > Maybe (b) is the way to go. When I wrote this patch, I did not realize that > only CMake had the ENABLE_JIT option. Filip, Ossy, what do you prefer?
(In reply to comment #11) > Ah, I didn't realize that list of options was used for anything besides the > default values. I think we should probably bring that back, as otherwise you > won't be able to use build-webkit on various architectures. > > Options: (a) convert the XCode build to use ENABLE_JIT so that we can > restore the option to FeatureList.pm, or (b) revert this patch and instead > remove the ENABLE_JIT option from the CMake build. > > Maybe (b) is the way to go. When I wrote this patch, I did not realize that > only CMake had the ENABLE_JIT option. I would prefer option (a), I think ENABLE_JIT is more descriptive name than ENABLE_LLINT_C_LOOP. Additionally they are the opposite of each other, why should we have two different option? So I think we should use ENABLE_JIT everywhere, only the following files should be changed: - Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:464 - */FeatureDefines.xcconfig - Platform.h:762 - build-jsc
Aye-aye, see bug #144304.