Bug 209322 – Update check for aarch64

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Michael Catanzaro]

/* CPU(ARM64) - Apple */
#if (defined(__arm64__) && defined(__APPLE__)) || defined(__aarch64__)
#define WTF_CPU_ARM64 1
#define WTF_CPU_KNOWN 1

#if defined(__arm64e__)
#define WTF_CPU_ARM64E 1
#endif
#endif


/* CPU(ARM64) - Apple */ is clearly a stale comment, since we're using CPU(ARM64) for Linux aarch64. Looks like the __APPLE__ check is obsolete. Let's see if removing it breaks anything?

[Michael Catanzaro]

Created attachment 394048 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 394048 [details]
Patch

I seem to recall there were competing 64-bit ARM instruction sets when it originally came out--I'm not sure whether those got resolved, or if they're close enough that both of these can be defined as CPU(ARM64).

I'll CC Mark Lam as he might remember.

[Mark Lam]

(In reply to David Kilzer (:ddkilzer) from comment #2)
> Comment on attachment 394048 [details]
> Patch
> 
> I seem to recall there were competing 64-bit ARM instruction sets when it
> originally came out--I'm not sure whether those got resolved, or if they're
> close enough that both of these can be defined as CPU(ARM64).
> 
> I'll CC Mark Lam as he might remember.

I don't remember such a difference.  On Apple's side, I believe this change is fine.  I tested on all relevant build targets.  If this also works for linux, I'm fine with the change.

[Mark Lam]

Comment on attachment 394048 [details]
Patch

r=me

[EWS]

Committed r259332: <https://trac.webkit.org/changeset/259332>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394048 [details].

[Radar WebKit Bug Importer]

<rdar://problem/61135818>

[Saam Barati]

Comment on attachment 394048 [details]
Patch

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

> Source/WTF/wtf/PlatformCPU.h:116
> +#if defined(__arm64__) || defined(__aarch64__)

Doesn’t this also need to be done in bmalloc?

[Michael Catanzaro]

Yeah... but without a warning comment that the files need to be synced, how are devs to know? :(

[Michael Catanzaro]

(In reply to Mark Lam from comment #3)
> I don't remember such a difference.  On Apple's side, I believe this change
> is fine.  I tested on all relevant build targets.  If this also works for
> linux, I'm fine with the change.

This won't change anything on Linux. It's just cleaning up a condition that doesn't seem to be doing anything.

[Michael Catanzaro]

Created attachment 395165 [details]
Patch

[Mark Lam]

Comment on attachment 394048 [details]
Patch

Let’s not obsolete this first patch since it was landed, and not replaced.

[EWS]

Committed r259344: <https://trac.webkit.org/changeset/259344>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395165 [details].

[Saam Barati]

(In reply to Michael Catanzaro from comment #8)
> Yeah... but without a warning comment that the files need to be synced, how
> are devs to know? :(

Yeah it’s not great, but a lot of things in bmalloc and WTF need to be synced.