Bug 190878 - Add BPLATFORM(IOS_FAMILY)
Summary: Add BPLATFORM(IOS_FAMILY)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: bmalloc (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-24 10:14 PDT by Alexey Proskuryakov
Modified: 2018-10-24 14:02 PDT (History)
5 users (show)

See Also:

Attachments
proposed patch (8.00 KB, patch)
2018-10-24 10:19 PDT, Alexey Proskuryakov 		saam: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2018-10-24 10:14:19 PDT 
Just like in WTF, BPLATFORM(IOS) is a misnomer. Change that to strictly match iOS, and add BPLATFORM(IOS_FAMILY).
Comment 1 Alexey Proskuryakov 2018-10-24 10:19:03 PDT 
Created attachment 353038 [details]
proposed patch

Mostly mechanical. It may be worth verifying if BUSE_CHECK_NANO_MALLOC and GIGACAGE_ENABLED are defined correctly. I believe that I kept the behavior unchanged, but it's surprising that gigacage is only enabled on iOS proper.

 #if ((BOS(DARWIN) || BOS(LINUX)) && \
-    (BCPU(X86_64) || (BCPU(ARM64) && !defined(__ILP32__) && (!BPLATFORM(IOS) || __IPHONE_OS_VERSION_MIN_REQUIRED >= 110300))))
+    (BCPU(X86_64) || (BCPU(ARM64) && !defined(__ILP32__) && (!BPLATFORM(IOS_FAMILY) || BPLATFORM(IOS)))))
 #define GIGACAGE_ENABLED 1
Comment 2 Saam Barati 2018-10-24 13:09:11 PDT 
Comment on attachment 353038 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=353038&action=review

r=me

> Source/bmalloc/bmalloc/BPlatform.h:-241
> -#if ((BPLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 120000) || (BPLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 50000) || (BPLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 120000)) \

Do we not care about building for  iOS 11 anymore? I have no use for this, but does our perf infrastructure care?
Comment 3 Saam Barati 2018-10-24 13:09:28 PDT 
(In reply to Alexey Proskuryakov from comment #1)
> Created attachment 353038 [details]
> proposed patch
> 
> Mostly mechanical. It may be worth verifying if BUSE_CHECK_NANO_MALLOC and
> GIGACAGE_ENABLED are defined correctly. I believe that I kept the behavior
> unchanged, but it's surprising that gigacage is only enabled on iOS proper.
> 
>  #if ((BOS(DARWIN) || BOS(LINUX)) && \
> -    (BCPU(X86_64) || (BCPU(ARM64) && !defined(__ILP32__) &&
> (!BPLATFORM(IOS) || __IPHONE_OS_VERSION_MIN_REQUIRED >= 110300))))
> +    (BCPU(X86_64) || (BCPU(ARM64) && !defined(__ILP32__) &&
> (!BPLATFORM(IOS_FAMILY) || BPLATFORM(IOS)))))
>  #define GIGACAGE_ENABLED 1

They look correct to me
Comment 4 Saam Barati 2018-10-24 13:14:52 PDT 
Comment on attachment 353038 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=353038&action=review

> Source/bmalloc/bmalloc/BPlatform.h:56
> +#if TARGET_OS_IOS

This is true for iPhone/iPad?
Comment 5 Alexey Proskuryakov 2018-10-24 13:59:13 PDT 
> Do we not care about building for  iOS 11 anymore? I have no use for this, but does our perf infrastructure care?

No, iOS 11 build is not longer supported by trunk. I'll be cleaning up more of the checks for older OS versions next.

> > +#if TARGET_OS_IOS
> 
> This is true for iPhone/iPad?

Yes, these and only these (plus simulators of course).
Comment 6 Alexey Proskuryakov 2018-10-24 14:01:39 PDT 
Committed http://trac.webkit.org/r237399
Comment 7 Radar WebKit Bug Importer 2018-10-24 14:02:53 PDT 
<rdar://problem/45532352>