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

Fangrui Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 24 16:05:50 PDT 2023


MaskRay 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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158641



More information about the cfe-commits mailing list