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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
