Since Android API version 9 the Android libm has had the sincos functions, so they should be recognised as libcalls and sincos optimisation should be applied.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/CodeGen/TargetLoweringBase.cpp | ||
---|---|---|
165 ↗ | (On Diff #165246) | I think there should be SDK version.. sdk 28. |
lib/CodeGen/TargetLoweringBase.cpp | ||
---|---|---|
165 ↗ | (On Diff #165246) | I do actually mean 9 here. It looks like sincos was added by https://android.googlesource.com/platform/bionic/+/ddd235bd9c264f08dee7887e210d61ca2351cf86 and https://android.googlesource.com/platform/bionic/+/9946750609c858dad0150da55645c4331392cf0d which first appeared in Android 2.3 which is API version 9. |
lib/CodeGen/TargetLoweringBase.cpp | ||
---|---|---|
165 ↗ | (On Diff #165246) | Ah sorry, I thought you mean Android 9 Pie :D |
Someone just pointed out to me that even in Android P [1], libm's sincos() just called sin() followed by cos() so this optimization won't be beneficial. It was changed in March 2018 [2] but is not a part of any release yet.
Unless there's any objection, let's revert this change and reintroduce it at a future time, enabling it from the next Android release forward.
[1] https://android.googlesource.com/platform/bionic/+/pie-release/libm/sincos.c
[2] https://android-review.googlesource.com/c/platform/bionic/+/681682/
I just wanted to follow up here.
tl;dr - The patch is actually ok as-is, and does not need to be reverted.
At worst, this results in an extra call to sincos() (that then calls sin() followed by cos()) on older Android releases. For all newer releases, it will indeed use the optimized path. The only drawback to this patch is that it prevents new toolchains from being used to compile older versions of the Android platform (i.e. the core operating system code). This happens all the way up to Android Pie. Since AOSP has moved on, however, I don't feel that this is a big concern. Partners generally don't update toolchains for previously released OS versions, and newer OS versions (with newer toolchains) don't experience this problem.