Page MenuHomePhabricator

[TargetLowering] Android has sincos functions

Authored by john.brawn on Sep 13 2018, 4:27 AM.



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.

Diff Detail


Event Timeline

john.brawn created this revision.Sep 13 2018, 4:27 AM
xbolva00 added inline comments.
165 ↗(On Diff #165246)

I think there should be SDK version.. sdk 28.

john.brawn added inline comments.Sep 13 2018, 5:11 AM
165 ↗(On Diff #165246)

I do actually mean 9 here. It looks like sincos was added by and which first appeared in Android 2.3 which is API version 9.

xbolva00 added inline comments.Sep 13 2018, 5:31 AM
165 ↗(On Diff #165246)

Ah sorry, I thought you mean Android 9 Pie :D

pirama accepted this revision.Sep 14 2018, 10:00 AM
pirama added subscribers: enh, danalbert.
This revision is now accepted and ready to land.Sep 14 2018, 10:00 AM
This revision was automatically updated to reflect the committed changes.

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.


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.