[Clang] Enable IC/IF mode for __ibm128

[qiucf]

Summary

As for 128-bit floating points on PPC, compiler should have three machine modes:

• IFmode, Always IBM extended double
• KFmode, Always IEEE 754R 128-bit floating point (implemented in D80374)
• TFmode, Matches the default for long double
  • If -mabi=ieeelongdouble, TFmode is IEEE 754R 128-bit floating point, and the L/l suffixes produces IEEE 754R constants
  • If -mabi=ibmlongdouble, TFmode is IBM extended double, and the L/l suffixes produces IBM extended double constants

[rjmccall]

clang/include/clang/AST/ASTContext.h

44

It's a shame to have to do this in such a pervasively-included header as ASTContext.h. Can we make TargetInfo::RealType an enum class at namespace scope, so that we can just forward-declare it in this file?

This would also be a good opportunity to rename the enum to something like ModeTypeKind.

If we do that, it should be a separate patch, so that this can just be the __ibm128 functionality change.

clang/lib/Basic/TargetInfo.cpp

300

__ibm128 is target-specific, right? So this should return NoFloat if someone requests IF or IC and the target doesn't support it, just like we do when the target doesn't support KF.

[qiucf]

Split and rebase upon D111391.

[aaron.ballman]

I'd appreciate some codegen tests for the new functionality as well (so we're testing not just "do we accept it" but also "does it work" as well), but this looks correct to me.

[rjmccall]

I agree. It doesn't have to be a CodeGen test, just anything that directly verifies that we get the right type, since I think those calls can succeed due to promotion.

[qiucf]

Add codegen test to show generated type in IR.

[rjmccall]

Thanks, LGTM

[eandrews]

This has changed the behavior for -

// Define __complex128 type corresponding to __float128 (as in GCC headers).
typedef _Complex float __attribute__((mode(TC))) __complex128;

void check() {
// CHECK: alloca { fp128, fp128 }   <----- Fails because alloca { x86_fp80, x86_fp80 } is generated instead
__complex128 tmp;
}

In TargetInfo.cpp, we used to check long double format to decide whether to return LongDouble or Float128. This patch doesn't, and so the return value has changed from FloatModeKind::Float128 to clang::FloatModeKind::LongDouble (ExplicitType). Should we be checking the format?

[rjmccall]

Oh, yes, I think this should be preserving the old logic there and just adding a new clause for explicit requests for ibm128, right?

[eandrews]

In D109950#3097161, @rjmccall wrote:

Oh, yes, I think this should be preserving the old logic there and just adding a new clause for explicit requests for ibm128, right?

I think the old logic should be preserved yes. However, I'm not sure if this patch exposes an existing bug. LongDoubleFormat here is llvm::semX87DoubleExtended.

In APFloat.cpp, it is defined as -

static const fltSemantics semX87DoubleExtended = {16383, -16382, 64, 80};

i.e. 80 bit size. Is this the right format for complex type specified using mode 'TC'? I am not very familiar with floating point semantics, so I thought I would ask.

[rjmccall]

In D109950#3097544, @eandrews wrote:

In D109950#3097161, @rjmccall wrote:

Oh, yes, I think this should be preserving the old logic there and just adding a new clause for explicit requests for ibm128, right?

I think the old logic should be preserved yes. However, I'm not sure if this patch exposes an existing bug. LongDoubleFormat here is llvm::semX87DoubleExtended.

In APFloat.cpp, it is defined as -

static const fltSemantics semX87DoubleExtended = {16383, -16382, 64, 80};

i.e. 80 bit size. Is this the right format for complex type specified using mode 'TC'? I am not very familiar with floating point semantics, so I thought I would ask.

The existing logic is that TF and TC should based on long double if the target has a 128-bit long double format, and otherwise they should be based on float128_t if that exists, and otherwise they're invalid. We should definitely not be using long double when it's not a 128-bit format, like if it's the 80-bit x87 format.

I do wonder whether that's the right priority between float128_t and a double-double long double. I think that particular configuration only exists on certain PPC targets; does anyone know what GCC does?

[eandrews]

In D109950#3097652, @rjmccall wrote:

In D109950#3097544, @eandrews wrote:

In D109950#3097161, @rjmccall wrote:

Oh, yes, I think this should be preserving the old logic there and just adding a new clause for explicit requests for ibm128, right?

I do wonder whether that's the right priority between float128_t and a double-double long double. I think that particular configuration only exists on certain PPC targets; does anyone know what GCC does?

I don't know what GCC does and am not really sure how to go about figuring it out. I've uploaded a patch preserving old logic for review here - https://reviews.llvm.org/D112975. If I should do something different, please let me know. Thank you for your input!

[rjmccall]

Thanks. @hubert.reinterpretcast, @qiucf, can you verify that other compilers for PPC follow the logic for TF / TC that we now have with Elizabeth's patch? There are three different type specifiers (long double, __ibm128, and float128_t) which represent formally distinct types, and IIUC there are a bunch of different flags and target options that change the meaning of long double and/or disable/enable __ibm128 and float128_t. We care about exactly which type is produced by the mode attribute, so you'll have to do something which is sensitive to the exact formal type, like _Generic or a C++ template or doing a pointer conversion without a cast; checking code generation will only tell us the underlying format.

[qiucf]

In D109950#3101740, @rjmccall wrote:

Thanks. @hubert.reinterpretcast, @qiucf, can you verify that other compilers for PPC follow the logic for TF / TC that we now have with Elizabeth's patch? There are three different type specifiers (long double, __ibm128, and float128_t) which represent formally distinct types, and IIUC there are a bunch of different flags and target options that change the meaning of long double and/or disable/enable __ibm128 and float128_t. We care about exactly which type is produced by the mode attribute, so you'll have to do something which is sensitive to the exact formal type, like _Generic or a C++ template or doing a pointer conversion without a cast; checking code generation will only tell us the underlying format.

typedef float __attribute__((mode(TF))) __tf128;
typedef float __attribute__((mode(IF))) __if128;

void foo() { printf(__func__); }
void bar() { printf(__func__); }
#define TEST(x) _Generic(x, __ibm128: foo, default: bar)()

int main() {
  __if128 x;
  __tf128 y;
  TEST(x);
  TEST(y);
}

// GCC (long double=ieee): foobar
// GCC (long double=ibm): foofoo
// Clang (both): foobar

As discussed in https://reviews.llvm.org/D93377#inline-874356 , GCC and Clang handles them differently: in GCC __ibm128 or __float128 is alias to long double under appropriate option, but Clang sees them as different types. While for explicit mode, Clang thinks float __attribute__((mode(IF))) and __ibm128 are the same type. The patch doesn't change the behavior.

For x86 __complex128 check(__complex128 a, __complex128 b) { return a+b; }, GCC also generates two ___addtf3.

clang/include/clang/AST/ASTContext.h

 #include "clang/Basic/Linkage.h"
 #include "clang/Basic/NoSanitizeList.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/ProfileList.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetCXXABI.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/XRayLists.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/MapVector.h"

 class ObjCProtocolDecl;
 class ObjCTypeParamDecl;
 class OMPTraitInfo;
 struct ParsedTargetAttr;
 class Preprocessor;
 class Stmt;
 class StoredDeclsMap;
 class TargetAttr;
-class TargetInfo;
 class TemplateDecl;
 class TemplateParameterList;
 class TemplateTemplateParmDecl;
 class TemplateTypeParmDecl;
 class UnresolvedSetIterator;
 class UsingShadowDecl;
 class VarTemplateDecl;
 class VTableContextBase;

   /// bitwidth, signed/unsigned.
   /// Returns empty type if there is no appropriate target types.
   QualType getIntTypeForBitwidth(unsigned DestWidth,
                                  unsigned Signed) const;
 
   /// getRealTypeForBitwidth -
   /// sets floating point QualTy according to specified bitwidth.
   /// Returns empty type if there is no appropriate target types.
-  QualType getRealTypeForBitwidth(unsigned DestWidth, bool ExplicitIEEE) const;
+  QualType getRealTypeForBitwidth(unsigned DestWidth,
+                                  TargetInfo::RealType ExplicitType) const;
 
   bool AtomicUsesUnsupportedLibcall(const AtomicExpr *E) const;
 
   const LangOptions& getLangOpts() const { return LangOpts; }
 
   // If this condition is false, typo correction must be performed eagerly
   // rather than delayed in many places, as it makes use of dependent types.
   // the condition is false for clang's C-only codepath, as it doesn't support

clang/include/clang/Basic/TargetInfo.h

 
   /// Return the smallest integer type with at least the specified width.
   virtual IntType getLeastIntTypeByWidth(unsigned BitWidth,
                                          bool IsSigned) const;
 
   /// Return floating point type with specified width. On PPC, there are
   /// three possible types for 128-bit floating point: "PPC double-double",
   /// IEEE 754R quad precision, and "long double" (which under the covers
-  /// is represented as one of those two). At this time, there is no support
-  /// for an explicit "PPC double-double" type (i.e. __ibm128) so we only
-  /// need to differentiate between "long double" and IEEE quad precision.
-  RealType getRealTypeByWidth(unsigned BitWidth, bool ExplicitIEEE) const;
+  /// is represented as one of those two).
+  RealType getRealTypeByWidth(unsigned BitWidth, RealType ExplicitType) const;
 
   /// Return the alignment (in bits) of the specified integer type enum.
   ///
   /// For example, SignedInt -> getIntAlign().
   unsigned getTypeAlign(IntType T) const;
 
   /// Returns true if the type is signed; false otherwise.
   static bool isTypeSigned(IntType T);

clang/lib/AST/ASTContext.cpp

   if (!QualTy && DestWidth == 128)
     return Signed ? Int128Ty : UnsignedInt128Ty;
   return QualTy;
 }
 
 /// getRealTypeForBitwidth -
 /// sets floating point QualTy according to specified bitwidth.
 /// Returns empty type if there is no appropriate target types.
-QualType ASTContext::getRealTypeForBitwidth(unsigned DestWidth,
-                                            bool ExplicitIEEE) const {
+QualType
+ASTContext::getRealTypeForBitwidth(unsigned DestWidth,
+                                   TargetInfo::RealType ExplicitType) const {
   TargetInfo::RealType Ty =
-      getTargetInfo().getRealTypeByWidth(DestWidth, ExplicitIEEE);
+      getTargetInfo().getRealTypeByWidth(DestWidth, ExplicitType);
   switch (Ty) {
   case TargetInfo::Float:
     return FloatTy;
   case TargetInfo::Double:
     return DoubleTy;
   case TargetInfo::LongDouble:
     return LongDoubleTy;
   case TargetInfo::Float128:

clang/lib/Basic/TargetInfo.cpp

     return IsSigned ? SignedInt : UnsignedInt;
   if (getLongWidth() >= BitWidth)
     return IsSigned ? SignedLong : UnsignedLong;
   if (getLongLongWidth() >= BitWidth)
     return IsSigned ? SignedLongLong : UnsignedLongLong;
   return NoInt;
 }
 
-TargetInfo::RealType TargetInfo::getRealTypeByWidth(unsigned BitWidth,
-                                                    bool ExplicitIEEE) const {
+TargetInfo::RealType
+TargetInfo::getRealTypeByWidth(unsigned BitWidth, RealType ExplicitType) const {
   if (getFloatWidth() == BitWidth)
     return Float;
   if (getDoubleWidth() == BitWidth)
     return Double;
 
   switch (BitWidth) {
   case 96:
     if (&getLongDoubleFormat() == &llvm::APFloat::x87DoubleExtended())
       return LongDouble;
     break;
   case 128:
     // The caller explicitly asked for an IEEE compliant type but we still
     // have to check if the target supports it.
-    if (ExplicitIEEE)
+    if (ExplicitType == Float128)
       return hasFloat128Type() ? Float128 : NoFloat;
-    if (&getLongDoubleFormat() == &llvm::APFloat::PPCDoubleDouble() ||
-        &getLongDoubleFormat() == &llvm::APFloat::IEEEquad())
-      return LongDouble;
-    if (hasFloat128Type())
-      return Float128;
+    if (ExplicitType == Ibm128 || ExplicitType == LongDouble)
+      return ExplicitType;
     break;
   }
 
   return NoFloat;
 }
 
 /// getTypeAlign - Return the alignment (in bits) of the specified integer type
 /// enum. For example, SignedInt -> getIntAlign().

clang/lib/Sema/SemaDeclAttr.cpp

   Diag(RD->getDefinition()->getLocation(), diag::note_defined_here) << RD;
   return true;
 }
 
 /// parseModeAttrArg - Parses attribute mode string and returns parsed type
 /// attribute.
 static void parseModeAttrArg(Sema &S, StringRef Str, unsigned &DestWidth,
                              bool &IntegerMode, bool &ComplexMode,
-                             bool &ExplicitIEEE) {
+                             TargetInfo::RealType &ExplicitType) {
   IntegerMode = true;
   ComplexMode = false;
+  ExplicitType = TargetInfo::NoFloat;
   switch (Str.size()) {
   case 2:
     switch (Str[0]) {
     case 'Q':
       DestWidth = 8;
       break;
     case 'H':
       DestWidth = 16;
       break;
     case 'S':
       DestWidth = 32;
       break;
     case 'D':
       DestWidth = 64;
       break;
     case 'X':
       DestWidth = 96;
       break;
     case 'K': // KFmode - IEEE quad precision (__float128)
-      ExplicitIEEE = true;
+      ExplicitType = TargetInfo::Float128;
       DestWidth = Str[1] == 'I' ? 0 : 128;
       break;
     case 'T':
-      ExplicitIEEE = false;
+      ExplicitType = TargetInfo::LongDouble;
       DestWidth = 128;
       break;
+    case 'I':
+      ExplicitType = TargetInfo::Ibm128;
+      DestWidth = Str[1] == 'I' ? 0 : 128;
+      break;
     }
     if (Str[1] == 'F') {
       IntegerMode = false;
     } else if (Str[1] == 'C') {
       IntegerMode = false;
       ComplexMode = true;
     } else if (Str[1] != 'I') {
       DestWidth = 0;

                        IdentifierInfo *Name, bool InInstantiation) {
   StringRef Str = Name->getName();
   normalizeName(Str);
   SourceLocation AttrLoc = CI.getLoc();
 
   unsigned DestWidth = 0;
   bool IntegerMode = true;
   bool ComplexMode = false;
-  bool ExplicitIEEE = false;
+  TargetInfo::RealType ExplicitType = TargetInfo::NoFloat;
   llvm::APInt VectorSize(64, 0);
   if (Str.size() >= 4 && Str[0] == 'V') {
     // Minimal length of vector mode is 4: 'V' + NUMBER(>=1) + TYPE(>=2).
     size_t StrSize = Str.size();
     size_t VectorStringLength = 0;
     while ((VectorStringLength + 1) < StrSize &&
            isdigit(Str[VectorStringLength + 1]))
       ++VectorStringLength;
     if (VectorStringLength &&
         !Str.substr(1, VectorStringLength).getAsInteger(10, VectorSize) &&
         VectorSize.isPowerOf2()) {
       parseModeAttrArg(*this, Str.substr(VectorStringLength + 1), DestWidth,
-                       IntegerMode, ComplexMode, ExplicitIEEE);
+                       IntegerMode, ComplexMode, ExplicitType);
       // Avoid duplicate warning from template instantiation.
       if (!InInstantiation)
         Diag(AttrLoc, diag::warn_vector_mode_deprecated);
     } else {
       VectorSize = 0;
     }
   }
 
   if (!VectorSize)
     parseModeAttrArg(*this, Str, DestWidth, IntegerMode, ComplexMode,
-                     ExplicitIEEE);
+                     ExplicitType);
 
   // FIXME: Sync this with InitializePredefinedMacros; we need to match int8_t
   // and friends, at least with glibc.
   // FIXME: Make sure floating-point mappings are accurate
   // FIXME: Support XF and TF types
   if (!DestWidth) {
     Diag(AttrLoc, diag::err_machine_mode) << 0 /*Unknown*/ << Name;
     return;

   }
 
   QualType NewElemTy;
 
   if (IntegerMode)
     NewElemTy = Context.getIntTypeForBitwidth(DestWidth,
                                               OldElemTy->isSignedIntegerType());
   else
-    NewElemTy = Context.getRealTypeForBitwidth(DestWidth, ExplicitIEEE);
+    NewElemTy = Context.getRealTypeForBitwidth(DestWidth, ExplicitType);
 
   if (NewElemTy.isNull()) {
     Diag(AttrLoc, diag::err_machine_mode) << 1 /*Unsupported*/ << Name;
     return;
   }
 
   if (ComplexMode) {
     NewElemTy = Context.getComplexType(NewElemTy);

clang/test/Sema/attr-mode.c

 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); }
+typedef float w128ibm __attribute__((mode(IF)));
+typedef _Complex float cw128ibm __attribute__((mode(IC)));
+void f_ibm128_arg(__ibm128 *x);
+void f_ibm128_complex_arg(_Complex __ibm128 *x);
+void test_IFtype(w128ibm *a) { f_ibm128_arg(a); }
+void test_ICtype(cw128ibm *a) { f_ibm128_complex_arg(a); }
 #elif TEST_F128_PPC64
 typedef int invalid_7 __attribute((mode(KF))); // expected-error{{type of machine mode does not match type of base type}}
 typedef int invalid_8 __attribute((mode(KI))); // expected-error{{unknown machine mode}}
 typedef _Complex float cf128 __attribute__((mode(KC)));
 typedef float f128 __attribute__((mode(KF)));
 void f_f128_arg(__float128 *x);
 void f_f128_complex_arg(_Complex __float128 *x);
 void test_KFtype(f128 *a) { f_f128_arg(a); }