Correct x86_64 fp128 calling convention

[rnk]

Summary

These changes are for Android x86_64 targets to be compatible
with current Android g++ and conform to AMD64 ABI.

https://llvm.org/bugs/show_bug.cgi?id=23897

• Return type of long double (fp128) should be fp128, not x86_fp80.

• Vararg of long double (fp128) could be in register and overflowed to memory.

https://llvm.org/bugs/show_bug.cgi?id=24111

• Return value of long double (fp128) _Complex should be in memory like a structure of {fp128,fp128}.

[majnemer]

Please split this into two patches: one for the CC change and another for the mangling change.

[rnk]

We really should not use the general LLVM IR type conversion machinery to decide how we are classifying our arguments. There are existing instances of us doing this, but we should strive to eliminate them. I think the right approach is probably to check getLongDoubleFormat(), see if it is APFloat::IEEEquad or APFloat::x86DoubleExtended, and pick memory, sse, or x87 classifications based on that.

[majnemer]

lib/CodeGen/TargetInfo.cpp

1974–1989 ↗

(On Diff #30416)

I'd phrase this as:

} else if (ET == getContext().DoubleTy) {
  Lo = Hi = SSE;
} else if (ET == getContext().LongDoubleTy) {
  const llvm::fltSemantics *LDF = &getTarget().getLongDoubleFormat();
  if (LDF == &llvm::APFloat::IEEEquad)
    Current = Memory;
  else if (LDF == &llvm::APFloat::x87DoubleExtended)
    Current = ComplexX87;
  else if (LDF == &llvm::APFloat::IEEEdouble)
    Lo = Hi = SSE;
  else
    llvm_unreachable("unexpected long double representation!");
}

[chh]

Separated the name mangling change into http://reviews.llvm.org/D11466.
I will fix the CC part in this patch soon.

[chh]

Change conditions from

CGT.ConvertType(RetTy)->isFP128Ty()

to

BT->getKind() == BuiltinType::LongDouble &&
&getTarget().getLongDoubleFormat() == &llvm::APFloat::IEEEquad

Use %clang_cc1 in unit tests.
Some checked IL output still depends on -O.

[rnk]

lib/CodeGen/TargetInfo.cpp

1861–1866 ↗

(On Diff #30523)

Any reason we can't do the same fp classification here like we do below for complex types?

1975–1976 ↗

(On Diff #30523)

Nice, I like this simplification.

2528 ↗

(On Diff #30523)

Why can't we classify IEEEQuad long doubles as SSE in the usual classify() implementation?

2667 ↗

(On Diff #30523)

ditto

[chh]

I tried to make X86_64ABIInfo::classify to return (Lo=SSE, Hi=NoClass) for fp128 long double type, or (Lo=SSE, Hi=SSEUp). That is not enough, although making fp128 Complex type to "Memory" worked.

X86_64ABIInfo::classifyArgumentType and classifyReturnType
will classify fp128 type as "double" through the help of GetSSETypeAtOffset.
These two or three functions still need more changes to handle fp128.
So I used the special cases for fp128, which seemed simpler with lower risk.

The mapping to register classes is quite complicated to decide converted parameter or return types. Although AMD64 spec has lengthy rules written this way, the rules are quite difficult to understand the mapping of fp128 type.

Is there other way to simplify these classification functions?

lib/CodeGen/TargetInfo.cpp

1973–1987 ↗

(On Diff #30523)

Done.

[chh]

New svn diff after svn update.

[chh]

Ping. I hope the new synced diff will be easier to review.

[rnk]

In D11437#211165, @chh wrote:

I tried to make X86_64ABIInfo::classify to return (Lo=SSE, Hi=NoClass) for fp128 long double type, or (Lo=SSE, Hi=SSEUp). That is not enough, although making fp128 Complex type to "Memory" worked.

X86_64ABIInfo::classifyArgumentType and classifyReturnType
will classify fp128 type as "double" through the help of GetSSETypeAtOffset.
These two or three functions still need more changes to handle fp128.
So I used the special cases for fp128, which seemed simpler with lower risk.

The mapping to register classes is quite complicated to decide converted parameter or return types. Although AMD64 spec has lengthy rules written this way, the rules are quite difficult to understand the mapping of fp128 type.

Is there other way to simplify these classification functions?

I think the right approach is to classify as SSE+SSEUp. It didn't work for you because GetByteVectorType was turning fp128 types into <2 x double>, which will correctly use XMM registers, but is not the IR you wanted.

I have a patch that fixes the TODOs and simplifies the code, do you mind if I land that?

[rnk]

Comandeering so I can upload my diff.

[rnk]

• Update classify and GetByteVectorType

[chh]

Reid, thanks a lot for fixing my hacks!

I tried your new diff 31457 and it worked for Android libm and all my other tests.
I am still waiting for some review of the back end changes in http://reviews.llvm.org/D11438.
This patch can be submitted now or later with D11438.
Would you like to submit this one?

cfe/trunk/lib/CodeGen/TargetInfo.cpp

if (const BuiltinType *BT = Ty->getAs<BuiltinType>()) {
 
     if (k == BuiltinType::Void) {
       Current = NoClass;
     } else if (k == BuiltinType::Int128 || k == BuiltinType::UInt128) {
       Lo = Integer;
       Hi = Integer;
     } else if (k >= BuiltinType::Bool && k <= BuiltinType::LongLong) {
       Current = Integer;
-    } else if ((k == BuiltinType::Float || k == BuiltinType::Double) ||
-               (k == BuiltinType::LongDouble &&
-                getTarget().getTriple().isOSNaCl())) {
+    } else if (k == BuiltinType::Float || k == BuiltinType::Double) {
       Current = SSE;
     } else if (k == BuiltinType::LongDouble) {
+      const llvm::fltSemantics *LDF = &getTarget().getLongDoubleFormat();
+      if (LDF == &llvm::APFloat::IEEEquad) {
+        Lo = SSE;
+        Hi = SSEUp;
+      } else if (LDF == &llvm::APFloat::x87DoubleExtended) {
         Lo = X87;
         Hi = X87Up;
+      } else if (LDF == &llvm::APFloat::IEEEdouble) {
+        Current = SSE;
+      } else
+        llvm_unreachable("unexpected long double representation!");
     }
     // FIXME: _Decimal32 and _Decimal64 are SSE.
     // FIXME: _float128 and _Decimal128 are (SSE, SSEUp).
     return;
   }
 
   if (const EnumType *ET = Ty->getAs<EnumType>()) {
     // Classify the underlying integer type.
if (const ComplexType *CT = Ty->getAs<ComplexType>()) {
     QualType ET = getContext().getCanonicalType(CT->getElementType());
 
     uint64_t Size = getContext().getTypeSize(Ty);
     if (ET->isIntegralOrEnumerationType()) {
       if (Size <= 64)
         Current = Integer;
       else if (Size <= 128)
         Lo = Hi = Integer;
-    } else if (ET == getContext().FloatTy)
+    } else if (ET == getContext().FloatTy) {
       Current = SSE;
-    else if (ET == getContext().DoubleTy ||
-             (ET == getContext().LongDoubleTy &&
-              getTarget().getTriple().isOSNaCl()))
+    } else if (ET == getContext().DoubleTy) {
       Lo = Hi = SSE;
-    else if (ET == getContext().LongDoubleTy)
+    } else if (ET == getContext().LongDoubleTy) {
+      const llvm::fltSemantics *LDF = &getTarget().getLongDoubleFormat();
+      if (LDF == &llvm::APFloat::IEEEquad)
+        Current = Memory;
+      else if (LDF == &llvm::APFloat::x87DoubleExtended)
         Current = ComplexX87;
+      else if (LDF == &llvm::APFloat::IEEEdouble)
+        Lo = Hi = SSE;
+      else
+        llvm_unreachable("unexpected long double representation!");
+    }
 
     // If this complex type crosses an eightbyte boundary then it
     // should be split.
     uint64_t EB_Real = (OffsetBase) / 64;
     uint64_t EB_Imag = (OffsetBase + getContext().getTypeSize(ET)) / 64;
     if (Hi == NoClass && EB_Real != EB_Imag)
       Hi = Lo;
 

 /// register. Pick an LLVM IR type that will be passed as a vector register.
 llvm::Type *X86_64ABIInfo::GetByteVectorType(QualType Ty) const {
   // Wrapper structs/arrays that only contain vectors are passed just like
   // vectors; strip them off if present.
   if (const Type *InnerTy = isSingleElementStruct(Ty, getContext()))
     Ty = QualType(InnerTy, 0);
 
   llvm::Type *IRType = CGT.ConvertType(Ty);
-  if(isa<llvm::VectorType>(IRType))
+  if (isa<llvm::VectorType>(IRType) ||
+      IRType->getTypeID() == llvm::Type::FP128TyID)
     return IRType;
 
   // We couldn't find the preferred IR vector type for 'Ty'.
   uint64_t Size = getContext().getTypeSize(Ty);
   assert((Size == 128 || Size == 256) && "Invalid type found!");
 
   // Return a LLVM IR vector type based on the size of 'Ty'.
   return llvm::VectorType::get(llvm::Type::getDoubleTy(getVMContext()),

cfe/trunk/test/CodeGen/x86_64-fp128.c

+// RUN: %clang_cc1 -triple x86_64-linux-android -emit-llvm -O -o - %s \
+// RUN:    | FileCheck %s --check-prefix=ANDROID --check-prefix=CHECK
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -O -o - %s \
+// RUN:    | FileCheck %s --check-prefix=GNU --check-prefix=CHECK
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm -O -o - %s \
+// RUN:    | FileCheck %s --check-prefix=GNU --check-prefix=CHECK
+
+// Android uses fp128 for long double but other x86_64 targets use x86_fp80.
+
+long double dataLD = 1.0L;
+// ANDROID: @dataLD = global fp128 0xL00000000000000003FFF000000000000, align 16
+// GNU: @dataLD = global x86_fp80 0xK3FFF8000000000000000, align 16
+
+long double _Complex dataLDC = {1.0L, 1.0L};
+// ANDROID: @dataLDC = global { fp128, fp128 } { fp128 0xL00000000000000003FFF000000000000, fp128 0xL00000000000000003FFF000000000000 }, align 16
+// GNU: @dataLDC = global { x86_fp80, x86_fp80 } { x86_fp80 0xK3FFF8000000000000000, x86_fp80 0xK3FFF8000000000000000 }, align 16
+
+long double TestLD(long double x) {
+  return x * x;
+// ANDROID: define fp128 @TestLD(fp128 %x)
+// GNU: define x86_fp80 @TestLD(x86_fp80 %x)
+}
+
+long double _Complex TestLDC(long double _Complex x) {
+  return x * x;
+// ANDROID: define void @TestLDC({ fp128, fp128 }* {{.*}}, { fp128, fp128 }* {{.*}} %x)
+// GNU: define { x86_fp80, x86_fp80 } @TestLDC({ x86_fp80, x86_fp80 }* {{.*}} %x)
+}
+
+typedef __builtin_va_list va_list;
+
+int TestGetVarInt(va_list ap) {
+  return __builtin_va_arg(ap, int);
+// Since int can be passed in memory or in register there is a branch and a phi.
+// CHECK:   define i32 @TestGetVarInt(
+// CHECK:   br
+// CHECK:   load {{.*}} %overflow_arg_area_p
+// CHECK:   = phi
+// CHECK:   ret i32
+}
+
+double TestGetVarDouble(va_list ap) {
+  return __builtin_va_arg(ap, double);
+// Since double can be passed in memory or in register there is a branch and a phi.
+// CHECK:   define double @TestGetVarDouble(
+// CHECK:   br
+// CHECK:   load {{.*}} %overflow_arg_area_p
+// CHECK:   = phi
+// CHECK:   ret double
+}
+
+long double TestGetVarLD(va_list ap) {
+  return __builtin_va_arg(ap, long double);
+// fp128 can be passed in memory or in register, but x86_fp80 is in memory.
+// ANDROID: define fp128 @TestGetVarLD(
+// GNU:     define x86_fp80 @TestGetVarLD(
+// ANDROID: br
+// GNU-NOT: br
+// CHECK:   load {{.*}} %overflow_arg_area_p
+// ANDROID: = phi
+// GNU-NOT: = phi
+// ANDROID: ret fp128
+// GNU:     ret x86_fp80
+}
+
+long double _Complex TestGetVarLDC(va_list ap) {
+  return __builtin_va_arg(ap, long double _Complex);
+// Pair of fp128 or x86_fp80 are passed as struct in memory.
+// ANDROID:   define void @TestGetVarLDC({ fp128, fp128 }* {{.*}}, %struct.__va_list_tag*
+// GNU:       define { x86_fp80, x86_fp80 } @TestGetVarLDC(
+// CHECK-NOT: br
+// CHECK:     load {{.*}} %overflow_arg_area_p
+// CHECK-NOT: phi
+// ANDROID:   ret void
+// GNU:       ret { x86_fp80, x86_fp80 }
+}
+
+void TestVarArg(const char *s, ...);
+
+void TestPassVarInt(int x) {
+  TestVarArg("A", x);
+// CHECK: define void @TestPassVarInt(i32 %x)
+// CHECK: call {{.*}} @TestVarArg(i8* {{.*}}, i32 %x)
+}
+
+void TestPassVarFloat(float x) {
+  TestVarArg("A", x);
+// CHECK: define void @TestPassVarFloat(float %x)
+// CHECK: call {{.*}} @TestVarArg(i8* {{.*}}, double %
+}
+
+void TestPassVarDouble(double x) {
+  TestVarArg("A", x);
+// CHECK: define void @TestPassVarDouble(double %x)
+// CHECK: call {{.*}} @TestVarArg(i8* {{.*}}, double %x
+}
+
+void TestPassVarLD(long double x) {
+  TestVarArg("A", x);
+// ANDROID: define void @TestPassVarLD(fp128 %x)
+// ANDROID: call {{.*}} @TestVarArg(i8* {{.*}}, fp128 %x
+// GNU: define void @TestPassVarLD(x86_fp80 %x)
+// GNU: call {{.*}} @TestVarArg(i8* {{.*}}, x86_fp80 %x
+}
+
+void TestPassVarLDC(long double _Complex x) {
+  TestVarArg("A", x);
+// ANDROID:      define void @TestPassVarLDC({ fp128, fp128 }* {{.*}} %x)
+// ANDROID:      store fp128 %x.{{.*}}, fp128* %
+// ANDROID-NEXT: store fp128 %x.{{.*}}, fp128* %
+// ANDROID-NEXT: call {{.*}} @TestVarArg(i8* {{.*}}, { fp128, fp128 }* {{.*}} %
+// GNU:          define void @TestPassVarLDC({ x86_fp80, x86_fp80 }* {{.*}} %x)
+// GNU:          store x86_fp80 %x.{{.*}}, x86_fp80* %
+// GNU-NEXT:     store x86_fp80 %x.{{.*}}, x86_fp80* %
+// GNGNU-NEXT:   call {{.*}} @TestVarArg(i8* {{.*}}, { x86_fp80, x86_fp80 }* {{.*}} %
+}