RESOLVED FIXED Bug 220477
[JSC] Disable JITCage compile time in old iOS 
https://bugs.webkit.org/show_bug.cgi?id=220477
Summary [JSC] Disable JITCage compile time in old iOS
Yusuke Suzuki
Reported 2021-01-08 12:59:19 PST
[JSC] Disable JITCage compile time in old iOS
Attachments
Patch (2.57 KB, patch)
2021-01-08 12:59 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (3.08 KB, patch)
2021-01-08 13:06 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (3.08 KB, patch)
2021-01-08 13:08 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2021-01-08 12:59:46 PST
Created attachment 417292 [details] Patch
Darin Adler
Comment 2 2021-01-08 13:00:59 PST
Comment on attachment 417292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417292&action=review > Source/WTF/wtf/PlatformEnable.h:883 > -#if OS(DARWIN) && ENABLE(JIT) && ((USE(APPLE_INTERNAL_SDK) && CPU(ARM64E))) > +#if OS(DARWIN) && ENABLE(JIT) && ((USE(APPLE_INTERNAL_SDK) && CPU(ARM64E))) && defined(__IPHONE_OS_VERSION_MIN_REQUIRED) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 150000 This also disables JITCage on macOS. Is that correct?
Darin Adler
Comment 3 2021-01-08 13:01:41 PST
Comment on attachment 417292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417292&action=review >> Source/WTF/wtf/PlatformEnable.h:883 >> +#if OS(DARWIN) && ENABLE(JIT) && ((USE(APPLE_INTERNAL_SDK) && CPU(ARM64E))) && defined(__IPHONE_OS_VERSION_MIN_REQUIRED) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 150000 > > This also disables JITCage on macOS. Is that correct? Also, I suggest we remove the extra parentheses that make this expression harder to read.
Yusuke Suzuki
Comment 4 2021-01-08 13:02:28 PST
Comment on attachment 417292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417292&action=review >> Source/WTF/wtf/PlatformEnable.h:883 >> +#if OS(DARWIN) && ENABLE(JIT) && ((USE(APPLE_INTERNAL_SDK) && CPU(ARM64E))) && defined(__IPHONE_OS_VERSION_MIN_REQUIRED) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 150000 > > This also disables JITCage on macOS. Is that correct? Yes. And we are not enabling it in macOS (even if ENABLE(JIT_CAGE) is true, anyway, we will not use JITCage because Options::useJITCage becomes false for macOS). And our plan is not using it on macOS for now.
Yusuke Suzuki
Comment 5 2021-01-08 13:06:01 PST
Comment on attachment 417292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417292&action=review >>>> Source/WTF/wtf/PlatformEnable.h:883 >>>> +#if OS(DARWIN) && ENABLE(JIT) && ((USE(APPLE_INTERNAL_SDK) && CPU(ARM64E))) && defined(__IPHONE_OS_VERSION_MIN_REQUIRED) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 150000 >>> >>> This also disables JITCage on macOS. Is that correct? >> >> Also, I suggest we remove the extra parentheses that make this expression harder to read. > > Yes. And we are not enabling it in macOS (even if ENABLE(JIT_CAGE) is true, anyway, we will not use JITCage because Options::useJITCage becomes false for macOS). And our plan is not using it on macOS for now. And in macOS, it is not enabled even without this patch as expected (Options.cpp).
Yusuke Suzuki
Comment 6 2021-01-08 13:06:23 PST
Created attachment 417295 [details] Patch
Yusuke Suzuki
Comment 7 2021-01-08 13:06:40 PST
Comment on attachment 417295 [details] Patch Oops, I cleared the old patch accidentally.
Yusuke Suzuki
Comment 8 2021-01-08 13:08:04 PST
Created attachment 417296 [details] Patch
Darin Adler
Comment 9 2021-01-08 13:21:52 PST
Comment on attachment 417296 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417296&action=review > Source/WTF/wtf/PlatformEnable.h:883 > +#if OS(DARWIN) && ENABLE(JIT) && USE(APPLE_INTERNAL_SDK) && CPU(ARM64E) && defined(__IPHONE_OS_VERSION_MIN_REQUIRED) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 150000 We should probably move this to PlatformEnableCocoa.h and take out redundant checks like OS(DARWIN). iOS-specific flags are mostly in there. But maybe it needs to be here so it’s after ENABLE(JIT) is set?
Yusuke Suzuki
Comment 10 2021-01-08 13:58:32 PST
Comment on attachment 417296 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417296&action=review >> Source/WTF/wtf/PlatformEnable.h:883 >> +#if OS(DARWIN) && ENABLE(JIT) && USE(APPLE_INTERNAL_SDK) && CPU(ARM64E) && defined(__IPHONE_OS_VERSION_MIN_REQUIRED) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 150000 > > We should probably move this to PlatformEnableCocoa.h and take out redundant checks like OS(DARWIN). iOS-specific flags are mostly in there. But maybe it needs to be here so it’s after ENABLE(JIT) is set? Yes, we need ENABLE(JIT) to determine this status, so we cannot put it in EnableCocoa.h.
Yusuke Suzuki
Comment 11 2021-01-08 15:08:32 PST
Landing since the remaining EWS are not on ARM => (ENABLE(JIT_CAGE) was disabled before).
EWS
Comment 12 2021-01-08 16:01:03 PST
Committed r271332: <https://trac.webkit.org/changeset/271332> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417296 [details].
Radar WebKit Bug Importer
Comment 13 2021-01-08 16:02:19 PST
<rdar://problem/72947770>
Note You need to log in before you can comment on or make changes to this bug.