[TargetLowering] Android has sincos functions

[john.brawn]

Summary

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.

[xbolva00]

lib/CodeGen/TargetLoweringBase.cpp

165 ↗

(On Diff #165246)

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

[john.brawn]

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.

[xbolva00]

lib/CodeGen/TargetLoweringBase.cpp

165 ↗

(On Diff #165246)

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

[pirama]

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/

[srhines]

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.

llvm/trunk/lib/CodeGen/TargetLoweringBase.cpp

if (darwinHasSinCos(TT)) {
                               CallingConv::ARM_AAPCS_VFP);
       }
     }
   } else {
     setLibcallName(RTLIB::FPEXT_F16_F32, "__gnu_h2f_ieee");
     setLibcallName(RTLIB::FPROUND_F32_F16, "__gnu_f2h_ieee");
   }
 
-  if (TT.isGNUEnvironment() || TT.isOSFuchsia()) {
+  if (TT.isGNUEnvironment() || TT.isOSFuchsia() ||
+      (TT.isAndroid() && !TT.isAndroidVersionLT(9))) {
     setLibcallName(RTLIB::SINCOS_F32, "sincosf");
     setLibcallName(RTLIB::SINCOS_F64, "sincos");
     setLibcallName(RTLIB::SINCOS_F80, "sincosl");
     setLibcallName(RTLIB::SINCOS_F128, "sincosl");
     setLibcallName(RTLIB::SINCOS_PPCF128, "sincosl");
   }
 
   if (TT.isOSOpenBSD()) {

llvm/trunk/test/CodeGen/AArch64/arm64-sincos.ll

 ; RUN: llc < %s -mtriple=arm64-apple-ios7 | FileCheck %s --check-prefix CHECK-IOS
 ; RUN: llc < %s -mtriple=arm64-linux-gnu | FileCheck %s --check-prefix CHECK-LINUX
+; RUN: llc < %s -mtriple=arm64-linux-android | FileCheck %s --check-prefix CHECK-LINUX
 
 ; Combine sin / cos into a single call unless they may write errno (as
 ; captured by readnone attrbiute, controlled by clang -fmath-errno
 ; setting).
 ; rdar://12856873
 
 define float @test1(float %x) nounwind {
 entry:

llvm/trunk/test/CodeGen/ARM/sincos.ll

 ; RUN: llc < %s -mtriple=armv7-apple-ios6 -mcpu=cortex-a8 | FileCheck %s --check-prefix=NOOPT
 ; RUN: llc < %s -mtriple=armv7-apple-ios7 -mcpu=cortex-a8 | FileCheck %s --check-prefix=SINCOS
 ; RUN: llc < %s -mtriple=armv7-linux-gnu -mcpu=cortex-a8 | FileCheck %s --check-prefix=SINCOS-GNU
 ; RUN: llc < %s -mtriple=armv7-linux-gnueabi -mcpu=cortex-a8 \
 ; RUN:   --enable-unsafe-fp-math | FileCheck %s --check-prefix=SINCOS-GNU
+; RUN: llc < %s -mtriple=armv7-linux-android -mcpu=cortex-a8 | FileCheck %s --check-prefix=NOOPT-ANDROID
+; RUN: llc < %s -mtriple=armv7-linux-android9 -mcpu=cortex-a8 | FileCheck %s --check-prefix=SINCOS-GNU
 
 ; Combine sin / cos into a single call unless they may write errno (as
 ; captured by readnone attrbiute, controlled by clang -fmath-errno
 ; setting).
 ; rdar://12856873
 
 define float @test1(float %x) nounwind {
 entry:
 ; SINCOS-LABEL: test1:
 ; SINCOS: bl ___sincosf_stret
 
 ; SINCOS-GNU-LABEL: test1:
 ; SINCOS-GNU: bl sincosf
 
 ; NOOPT-LABEL: test1:
 ; NOOPT: bl _sinf
 ; NOOPT: bl _cosf
 
+; NOOPT-ANDROID-LABEL: test1:
+; NOOPT-ANDROID: bl sinf
+; NOOPT-ANDROID: bl cosf
+
   %call = tail call float @sinf(float %x) readnone
   %call1 = tail call float @cosf(float %x) readnone
   %add = fadd float %call, %call1
   ret float %add
 }
 
 define float @test1_errno(float %x) nounwind {
 entry:
 ; SINCOS-LABEL: test1_errno:
 ; SINCOS: bl _sinf
 ; SINCOS: bl _cosf
 
 ; SINCOS-GNU-LABEL: test1_errno:
 ; SINCOS-GNU: bl sinf
 ; SINCOS-GNU: bl cosf
 
 ; NOOPT-LABEL: test1_errno:
 ; NOOPT: bl _sinf
 ; NOOPT: bl _cosf
 
+; NOOPT-ANDROID-LABEL: test1_errno:
+; NOOPT-ANDROID: bl sinf
+; NOOPT-ANDROID: bl cosf
+
   %call = tail call float @sinf(float %x)
   %call1 = tail call float @cosf(float %x)
   %add = fadd float %call, %call1
   ret float %add
 }
 
 define double @test2(double %x) nounwind {
 entry:
 ; SINCOS-LABEL: test2:
 ; SINCOS: bl ___sincos_stret
 
 ; SINCOS-GNU-LABEL: test2:
 ; SINCOS-GNU: bl sincos
 
 ; NOOPT-LABEL: test2:
 ; NOOPT: bl _sin
 ; NOOPT: bl _cos
 
+; NOOPT-ANDROID-LABEL: test2:
+; NOOPT-ANDROID: bl sin
+; NOOPT-ANDROID: bl cos
+
   %call = tail call double @sin(double %x) readnone
   %call1 = tail call double @cos(double %x) readnone
   %add = fadd double %call, %call1
   ret double %add
 }
 
 define double @test2_errno(double %x) nounwind {
 entry:
 ; SINCOS-LABEL: test2_errno:
 ; SINCOS: bl _sin
 ; SINCOS: bl _cos
 
 ; SINCOS-GNU-LABEL: test2_errno:
 ; SINCOS-GNU: bl sin
 ; SINCOS-GNU: bl cos
 
 ; NOOPT-LABEL: test2_errno:
 ; NOOPT: bl _sin
 ; NOOPT: bl _cos
 
+; NOOPT-ANDROID-LABEL: test2_errno:
+; NOOPT-ANDROID: bl sin
+; NOOPT-ANDROID: bl cos
+
   %call = tail call double @sin(double %x)
   %call1 = tail call double @cos(double %x)
   %add = fadd double %call, %call1
   ret double %add
 }
 
 declare float  @sinf(float)
 declare double @sin(double)
 declare float @cosf(float)
 declare double @cos(double)