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

4176

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.

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;
     }
   }
 
   const char *getStaticInitSectionSpecifier() const override {
     return ".text.startup";
   }
 };
 
 // NetBSD Target
CallingConv getDefaultCallingConv(CallingConvMethodType MT) const override {
     return CC_C;
   }
 
   // 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 hasFloat128Type() const override {
+    return true;
+  }
+
   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;

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)));
 };