[PATCH] D158641: [AArch64][Android][DRAFT] Fix FMV ifunc resolver usage on old Android APIs.

[Stephen Hines via Phabricator via cfe-commits]

srhines added inline comments.


================
Comment at: compiler-rt/lib/builtins/cpu_model.c:1382
+    return;
+#if defined(__ANDROID__)
+  // ifunc resolvers don't have hwcaps in arguments on Android API lower
----------------
MaskRay wrote:
> enh wrote:
> > ilinpv wrote:
> > > MaskRay wrote:
> > > > I am unfamiliar with how Android ndk builds compiler-rt.
> > > > 
> > > > If `__ANDROID_API__ >= 30`, shall we use the regular Linux code path?
> > > I think that leads to shipping different compile-rt libraries depend on ANDROID_API. If this is an option to consider than runtime check android_get_device_api_level() < 30 can be replaced by `__ANDROID_API__ < 30`
> > depends what you mean... in 10 years or so, yes, no-one is likely to still care about the older API levels and we can just delete this. but until then, no, there's _one_ copy of compiler-rt that everyone uses, and although _OS developers_ don't need to support anything more than a couple of years old, most app developers are targeting far lower API levels than that (to maximize the number of possible customers).
> > 
> > TL;DR: "you could add that condition to the `#if`, but no-one would use it for a decade". (and i think the comment and `if` below should make it clear enough to future archeologists when this code block can be removed :-) )
> My thought was that people build Android with a specific `__ANDROID_API__`, and only systems >= this level are supported.
> ```
> #If __ANDROID_API__ < 30
> ...
> #endif
> ```
> 
> This code has a greater chance to be removed when it becomes obsoleted. The argument is similar to how we find obsoleted GCC workarounds.
Yes, the NDK currently just ships the oldest supported target API level for compiler-rt, while the Android platform build does have access to both the oldest supported target API level + the most recent target API level, so that we can make better use of features there.

Maybe I'm missing something, but it's feeling like the NDK users won't be able to make great use of FMV without us either bumping the minimum level or shipping multiple runtimes (and then using the #ifdefs properly here).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158641/new/

https://reviews.llvm.org/D158641