Enable __float128 on X86 and SystemZ

[nemanjai]

Summary

It would appear that gcc supports the __float128 type on X86_64. Systems with this architecture that I've seen have the GNU C++ library configured with _GLIBCXX_USE_FLOAT128 defined. This in turn is used in at least one header (type_traits) and there is one template specialization with this type in the header. As a result, Clang no longer compiles this header with -std=gnu++11.

This patch enables this type on X86_64 targets.

[hjl.tools]

Why not enable __float128 on Linux/i386, which is defined in
the current i386 psABI?

[nemanjai]

In D19125#401465, @hjl.tools wrote:

Why not enable __float128 on Linux/i386, which is defined in
the current i386 psABI?

I am certainly more than happy to do so. The issue is that I really don't know what set of targets have this enabled (i.e. I don't know how this config macro is determined). Can you please post a link to this i386 psABI so I can get agreement between all the reviewers?

[hjl.tools]

The current x86 psABIs are available from

https://github.com/hjl-tools/x86-psABI/wiki/X86-psABI

[bruno]

I agree with H.J Lu.

Looks like test/CodeGenCXX/float128-declarations.cpp deserves some x86 RUN lines too.

Some comments inline.

lib/Basic/Targets.cpp

4177

clang-format this one, looks like it could be a onliner like the other aboves, also place it below hasInt128Type().

test/Sema/128bitfloat.cpp

4

Better to have a x86-128bitfloat.cpp file instead, targetting i386/x86_64 with two different RUN lines. This ensures this feature gets tested when host isn't x86.

[nemanjai]

Updated the patch to address the comments:

1. Enabled __float128 for Linux targets (x86, x86_64 and systemz). Both x86 and x86_64 seem to have the support mentioned in the psABI and SystemZ has IEEE Quad precision support which this type is supposed to target.
2. Updated the test case for the targets that now have this type enabled. Please note that I have not updated the test case that expects diagnostics but rather the test case that tests this type in various declaration contexts.

[nemanjai]

lib/Basic/Targets.cpp

437

I am not sure this is the right way to do this, but it seems to encapsulate the requested semantics (i.e. Linux/i386...).

[nemanjai]

I am sorry to be pinging the reviewers already, but I'm wondering if we can get this patch sorted out and committed so I don't have to revert the patch that enables __float128 as it is a large patch and otherwise fine. I've already received a request to revert the initial patch and I suppose I'll have to do that if I can't get this one approved today.

[jonpa]

I tried running this on SystemZ, by reverting your revert commit.

I added

bool hasFloat128Type() const override {

return true;

}

in the SystemZTargetInfo definition.

test/Sema/128bitfloat.cpp fails, because of an expected_error: "__float128 is not supported on this target"

Otherwise, all tests seem to pass, including the test-suite.

[nemanjai]

test/Sema/128bitfloat.cpp

4

I think it'll be better if I change this to

#ifdef __FLOAT228__

[nemanjai]

A gentle reminder to all reviewers that this patch is holding up the addition of support for the __float128 type that I am hoping to commit. Please review.

[hjl.tools]

Should it be handled similar to int128? Only targets which support float128
override hasFloat128Type without adding HasFloat128.

[nemanjai]

In D19125#411929, @hjl.tools wrote:

Should it be handled similar to int128? Only targets which support float128
override hasFloat128Type without adding HasFloat128.

I implemented it this way so that it isn't necessarily tied to an architecture, but rather the specific architectures on Linux. My hope was to match your suggestion to enable it for Linux/i386. However, I am certainly open to suggestions as I really don't know exactly what the relationship is between __float128 and architecture/OS.

[nemanjai]

I really need to re-commit the changes for __float128 in the front end and this review is holding it up. Can I kindly ask the reviewers to have another look at this patch and give me some further direction?

[echristo]

Couple of inline comments, but this works for me provided answers are OK to the questions inline.

-eric

lib/Basic/Targets.cpp

437

Seems ok. What happened to the ppc64 lines? (I don't particularly like this solution, but it's fine for now)

test/Sema/128bitfloat.cpp

4

Modulo typo, but I think that works yeah.

[nemanjai]

I have made the update to the test case (without the typo). Also, added the FLOAT128 define where the target feature is set.
Committed revision 268898.

[uweigand]

GCC has never supported the __float128 type on SystemZ, because "long double" is already IEEE-128 on the platform. GCC only supports a separate __float128 type on platforms where "long double" is some other type (like x86 or ppc64).

For compatibility with GCC I've removed support for SystemZ again as rev. 348247.

include/clang/Basic/TargetInfo.h

class TargetInfo : public RefCountedBase<TargetInfo> {
   std::shared_ptr<TargetOptions> TargetOpts;
   llvm::Triple Triple;
 protected:
   // Target values set by the ctor of the actual target implementation.  Default
   // values are specified by the TargetInfo constructor.
   bool BigEndian;
   bool TLSSupported;
   bool NoAsmVariants;  // True if {|} are normal characters.
+  bool HasFloat128;
   unsigned char PointerWidth, PointerAlign;
   unsigned char BoolWidth, BoolAlign;
   unsigned char IntWidth, IntAlign;
   unsigned char HalfWidth, HalfAlign;
   unsigned char FloatWidth, FloatAlign;
   unsigned char DoubleWidth, DoubleAlign;
   unsigned char LongDoubleWidth, LongDoubleAlign, Float128Align;
   unsigned char LargeArrayMinWidth, LargeArrayAlign;
public:
   unsigned getLongLongAlign() const { return LongLongAlign; }
 
   /// \brief Determine whether the __int128 type is supported on this target.
   virtual bool hasInt128Type() const {
     return getPointerWidth(0) >= 64;
   } // FIXME
 
   /// \brief Determine whether the __float128 type is supported on this target.
-  virtual bool hasFloat128Type() const {
-    return false;
-  }
+  virtual bool hasFloat128Type() const { return HasFloat128; }
 
   /// \brief Return the alignment that is suitable for storing any
   /// object with a fundamental alignment requirement.
   unsigned getSuitableAlign() const { return SuitableAlign; }
 
   /// \brief Return the default alignment for __attribute__((aligned)) on
   /// this target, to be used if no alignment value is specified.
   unsigned getDefaultAlignForAttributeAligned() const {

lib/Basic/TargetInfo.cpp

 
 // TargetInfo Constructor.
 TargetInfo::TargetInfo(const llvm::Triple &T) : TargetOpts(), Triple(T) {
   // Set defaults.  Defaults are set for a 32-bit RISC platform, like PPC or
   // SPARC.  These should be overridden by concrete targets as needed.
   BigEndian = true;
   TLSSupported = true;
   NoAsmVariants = false;
+  HasFloat128 = false;
   PointerWidth = PointerAlign = 32;
   BoolWidth = BoolAlign = 8;
   IntWidth = IntAlign = 32;
   LongWidth = LongAlign = 32;
   LongLongWidth = LongLongAlign = 64;
   SuitableAlign = 64;
   DefaultAlignForAttributeAligned = 128;
   MinGlobalAlign = 0;

lib/Basic/Targets.cpp

LinuxTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
     switch (Triple.getArch()) {
     default:
       break;
     case llvm::Triple::ppc:
     case llvm::Triple::ppc64:
     case llvm::Triple::ppc64le:
       this->MCountName = "_mcount";
       break;
+    case llvm::Triple::x86:
+    case llvm::Triple::x86_64:
+    case llvm::Triple::systemz:
+      this->HasFloat128 = true;
+      break;
     }
   }
 
   const char *getStaticInitSectionSpecifier() const override {
     return ".text.startup";
   }
 };
 
class PPCTargetInfo : public TargetInfo {
   bool HasVSX;
   bool HasP8Vector;
   bool HasP8Crypto;
   bool HasDirectMove;
   bool HasQPX;
   bool HasHTM;
   bool HasBPERMD;
   bool HasExtDiv;
-  bool HasFloat128;
 
 protected:
   std::string ABI;
 
 public:
   PPCTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
     : TargetInfo(Triple), HasVSX(false), HasP8Vector(false),
       HasP8Crypto(false), HasDirectMove(false), HasQPX(false), HasHTM(false),
-      HasBPERMD(false), HasExtDiv(false), HasFloat128(false) {
+      HasBPERMD(false), HasExtDiv(false) {
     BigEndian = (Triple.getArch() != llvm::Triple::ppc64le);
     SimdDefaultAlign = 128;
     LongDoubleWidth = LongDoubleAlign = 128;
     LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble;
   }
 
   /// \brief Flags for architecture specific defines.
   typedef enum {
bool hasSjLjLowering() const override {
     return true;
   }
 
   bool useFloat128ManglingForLongDouble() const override {
     return LongDoubleWidth == 128 &&
            LongDoubleFormat == &llvm::APFloat::PPCDoubleDouble &&
            getTriple().isOSBinFormatELF();
   }
-  bool hasFloat128Type() const override {
-    return HasFloat128;
-  }
 };
 
 const Builtin::Info PPCTargetInfo::BuiltinInfo[] = {
 #define BUILTIN(ID, TYPE, ATTRS) \
   { #ID, TYPE, ATTRS, nullptr, ALL_LANGUAGES, nullptr },
 #define LIBBUILTIN(ID, TYPE, ATTRS, HEADER) \
   { #ID, TYPE, ATTRS, HEADER, ALL_LANGUAGES, nullptr },
 #include "clang/Basic/BuiltinsPPC.def"
public:
   // for x32 we need it here explicitly
   bool hasInt128Type() const override { return true; }
   unsigned getUnwindWordWidth() const override { return 64; }
   unsigned getRegisterWidth() const override { return 64; }
 
   bool validateGlobalRegisterVariable(StringRef RegName,
                                       unsigned RegSize,
                                       bool &HasSizeMismatch) const override {
     // rsp and rbp are the only 64-bit registers the x86 backend can currently
     // handle.
     if (RegName.equals("rsp") || RegName.equals("rbp")) {
       // Check that the register size is 64-bit.
       HasSizeMismatch = RegSize != 64;
       return true;
     }
 
     // Check if the register is a 32-bit register the backend can handle.

test/CodeGenCXX/float128-declarations.cpp

 // RUN: %clang_cc1 -emit-llvm -triple powerpc64-unknown-unknown \
 // RUN:   -target-feature +float128 -std=c++11 %s -o - | FileCheck %s
 // RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown \
 // RUN:   -target-feature +float128 -std=c++11 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple i386-unknown-linux-gnu -std=c++11 \
+// RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-X86
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-unknown-linux-gnu -std=c++11 \
+// RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-X86
+// RUN: %clang_cc1 -emit-llvm -triple systemz-unknown-linux-gnu -std=c++11 \
+// RUN:   %s -o - | FileCheck %s -check-prefix=CHECK-SYSZ
 //
-/*  Various contexts where type __float128 can appear. */
+/*  Various contexts where type __float128 can appear. The different check
+    prefixes are due to different mangling on X86 and different calling
+    convention on SystemZ. */
 
 /*  Namespace */
 namespace {
   __float128 f1n;
   __float128 f2n = 33.q;
   __float128 arr1n[10];
   __float128 arr2n[] = { 1.2q, 3.0q, 3.e11q };
   const volatile __float128 func1n(const __float128 &arg) {

 // CHECK-DAG: @_ZZ4mainE2s1 = private unnamed_addr constant %struct.S1 { fp128 0xL00000000000000004006080000000000 }
 // CHECK-DAG: store fp128 0xLF0AFD0EBFF292DCE42E0B38CDD83F26F, fp128* %f1l, align 16
 // CHECK-DAG: store fp128 0xL00000000000000008000000000000000, fp128* %f2l, align 16
 // CHECK-DAG: store fp128 0xLFFFFFFFFFFFFFFFF7FFEFFFFFFFFFFFF, fp128* %f3l, align 16
 // CHECK-DAG: store fp128 0xL0000000000000000BFFF000000000000, fp128* %f5l, align 16
 // CHECK-DAG: [[F4L:%[a-z0-9]+]] = load fp128, fp128* %f4l
 // CHECK-DAG: [[INC:%[a-z0-9]+]] = fadd fp128 [[F4L]], 0xL00000000000000003FFF000000000000
 // CHECK-DAG: store fp128 [[INC]], fp128* %f4l
+
+// CHECK-X86-DAG: @_ZN12_GLOBAL__N_13f1nE = internal global fp128 0xL00000000000000000000000000000000
+// CHECK-X86-DAG: @_ZN12_GLOBAL__N_13f2nE = internal global fp128 0xL00000000000000004004080000000000
+// CHECK-X86-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x fp128]
+// CHECK-X86-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x fp128] [fp128 0xL33333333333333333FFF333333333333, fp128 0xL00000000000000004000800000000000, fp128 0xL00000000000000004025176592E00000]
+// CHECK-X86-DAG: define internal fp128 @_ZN12_GLOBAL__N_16func1nERKg(fp128*
+// CHECK-X86-DAG: @f1f = global fp128 0xL00000000000000000000000000000000
+// CHECK-X86-DAG: @f2f = global fp128 0xL33333333333333334004033333333333
+// CHECK-X86-DAG: @arr1f = global [10 x fp128]
+// CHECK-X86-DAG: @arr2f = global [3 x fp128] [fp128 0xL3333333333333333BFFF333333333333, fp128 0xL0000000000000000C000800000000000, fp128 0xL0000000000000000C025176592E00000]
+// CHECK-X86-DAG: declare fp128 @_Z6func1fg(fp128)
+// CHECK-X86-DAG: define linkonce_odr void @_ZN2C1C2Eg(%class.C1* %this, fp128 %arg)
+// CHECK-X86-DAG: define linkonce_odr fp128 @_ZN2C16func2cEg(fp128 %arg)
+// CHECK-X86-DAG: define linkonce_odr fp128 @_Z6func1tIgET_S0_(fp128 %arg)
+// CHECK-X86-DAG: @_ZZ4mainE2s1 = private unnamed_addr constant %struct.S1 { fp128 0xL00000000000000004006080000000000 }
+// CHECK-X86-DAG: store fp128 0xLF0AFD0EBFF292DCE42E0B38CDD83F26F, fp128* %f1l, align 16
+// CHECK-X86-DAG: store fp128 0xL00000000000000008000000000000000, fp128* %f2l, align 16
+// CHECK-X86-DAG: store fp128 0xLFFFFFFFFFFFFFFFF7FFEFFFFFFFFFFFF, fp128* %f3l, align 16
+// CHECK-X86-DAG: store fp128 0xL0000000000000000BFFF000000000000, fp128* %f5l, align 16
+// CHECK-X86-DAG: [[F4L:%[a-z0-9]+]] = load fp128, fp128* %f4l
+// CHECK-X86-DAG: [[INC:%[a-z0-9]+]] = fadd fp128 [[F4L]], 0xL00000000000000003FFF000000000000
+// CHECK-X86-DAG: store fp128 [[INC]], fp128* %f4l
+
+// CHECK-SYSZ-DAG: @_ZN12_GLOBAL__N_13f1nE = internal global fp128 0xL00000000000000000000000000000000
+// CHECK-SYSZ-DAG: @_ZN12_GLOBAL__N_13f2nE = internal global fp128 0xL00000000000000004004080000000000
+// CHECK-SYSZ-DAG: @_ZN12_GLOBAL__N_15arr1nE = internal global [10 x fp128]
+// CHECK-SYSZ-DAG: @_ZN12_GLOBAL__N_15arr2nE = internal global [3 x fp128] [fp128 0xL33333333333333333FFF333333333333, fp128 0xL00000000000000004000800000000000, fp128 0xL00000000000000004025176592E00000]
+// CHECK-SYSZ-DAG: define internal void @_ZN12_GLOBAL__N_16func1nERKU10__float128(fp128*
+// CHECK-SYSZ-DAG: @f1f = global fp128 0xL00000000000000000000000000000000
+// CHECK-SYSZ-DAG: @f2f = global fp128 0xL33333333333333334004033333333333
+// CHECK-SYSZ-DAG: @arr1f = global [10 x fp128]
+// CHECK-SYSZ-DAG: @arr2f = global [3 x fp128] [fp128 0xL3333333333333333BFFF333333333333, fp128 0xL0000000000000000C000800000000000, fp128 0xL0000000000000000C025176592E00000]
+// CHECK-SYSZ-DAG: declare void @_Z6func1fU10__float128(fp128*
+// CHECK-SYSZ-DAG: define linkonce_odr void @_ZN2C1C2EU10__float128(%class.C1* %this, fp128*
+// CHECK-SYSZ-DAG: define linkonce_odr void @_ZN2C16func2cEU10__float128(fp128*
+// CHECK-SYSZ-DAG: define linkonce_odr void @_Z6func1tIU10__float128ET_S0_(fp128*
+// CHECK-SYSZ-DAG: @_ZZ4mainE2s1 = private unnamed_addr constant %struct.S1 { fp128 0xL00000000000000004006080000000000 }
+// CHECK-SYSZ-DAG: store fp128 0xLF0AFD0EBFF292DCE42E0B38CDD83F26F, fp128* %f1l, align 16
+// CHECK-SYSZ-DAG: store fp128 0xL00000000000000008000000000000000, fp128* %f2l, align 16
+// CHECK-SYSZ-DAG: store fp128 0xLFFFFFFFFFFFFFFFF7FFEFFFFFFFFFFFF, fp128* %f3l, align 16
+// CHECK-SYSZ-DAG: store fp128 0xL0000000000000000BFFF000000000000, fp128* %f5l, align 16
+// CHECK-SYSZ-DAG: [[F4L:%[a-z0-9]+]] = load fp128, fp128* %f4l
+// CHECK-SYSZ-DAG: [[INC:%[a-z0-9]+]] = fadd fp128 [[F4L]], 0xL00000000000000003FFF000000000000
+// CHECK-SYSZ-DAG: store fp128 [[INC]], fp128* %f4l

test/Sema/128bitfloat.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify -std=gnu++11 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
+#ifdef __x86_64__
+__float128 f;
+template<typename> struct __is_floating_point_helper {};
+template<> struct __is_floating_point_helper<__float128> {};
+int g(int x, __float128 *y) {
+  return x + *y;
+}
+
+// expected-no-diagnostics
+#else
 #if !defined(__STRICT_ANSI__)
 __float128 f;  // expected-error {{__float128 is not supported on this target}}
 // But this should work:
 template<typename> struct __is_floating_point_helper {};
 template<> struct __is_floating_point_helper<__float128> {};  // expected-error {{__float128 is not supported on this target}}
 
 // FIXME: This could have a better diag.
 int g(int x, __float128 *y) {  // expected-error {{__float128 is not supported on this target}}
   return x + *y;
 }
 
 #else
 __float128 f;  // expected-error {{__float128 is not supported on this target}}
 template<typename> struct __is_floating_point_helper {};
 template<> struct __is_floating_point_helper<__float128> {};  // expected-error {{__float128 is not supported on this target}}
 
 int g(int x, __float128 *y) {  // expected-error {{__float128 is not supported on this target}}
   return x + *y;
 }
 
 #endif
+#endif

test/Sema/attr-mode.c

 typedef unsigned int gcc_unwind_word __attribute__((mode(unwind_word)));
 int foo[sizeof(gcc_unwind_word) == 8 ? 1 : -1];
 void test_long_to_i64(long long* y) { f_i64_arg(y); }
 void test_long_to_ui64(unsigned long long* y) { f_ui64_arg(y); }
 #else
 void test_long_to_i64(long* y) { f_i64_arg(y); }
 void test_long_to_ui64(unsigned long* y) { f_ui64_arg(y); }
 #endif
-typedef          float f128ibm __attribute__ ((mode (TF)));     // expected-error{{unsupported machine mode 'TF'}}
+typedef          float f128ibm __attribute__ ((mode (TF)));
 #elif TEST_64BIT_PPC64
 typedef          float f128ibm __attribute__ ((mode (TF)));
 typedef _Complex float c128ibm __attribute__ ((mode (TC)));
 void f_ft128_arg(long double *x);
 void f_ft128_complex_arg(_Complex long double *x);
 void test_TFtype(f128ibm *a) { f_ft128_arg (a); }
 void test_TCtype(c128ibm *a) { f_ft128_complex_arg (a); }
 #else
 #error Unknown test architecture.
 #endif
 
 struct S {
   int n __attribute((mode(HI)));
 };