[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfae0dfa6421e: [Clang] Add __ibm128 type to represent 
ppc_fp128 (authored by qiucf).

Changed prior to commit:
  https://reviews.llvm.org/D93377?vs=369045=370878#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

Files:
  clang/bindings/python/clang/cindex.py
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/CodeGen/ibm128-cast.c
  clang/test/CodeGen/ibm128-unsupported.c
  clang/test/CodeGenCXX/ibm128-declarations.cpp
  clang/test/Sema/128bitfloat.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -60,6 +60,7 @@
 BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
+BTCASE(Ibm128);
 BTCASE(NullPtr);
 BTCASE(Overload);
 BTCASE(Dependent);
@@ -577,6 +578,7 @@
 TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
+TKIND(Ibm128);
 TKIND(NullPtr);
 TKIND(Overload);
 TKIND(Dependent);
Index: clang/test/Sema/128bitfloat.cpp
===
--- clang/test/Sema/128bitfloat.cpp
+++ clang/test/Sema/128bitfloat.cpp
@@ -20,7 +20,7 @@
   return x + *y;
 }
 
-// expected-no-diagnostics
+// expected-no-error {{__float128 is not supported on this target}}
 #else
 #if !defined(__STRICT_ANSI__)
 __float128 f;  // expected-error {{__float128 is not supported on this target}}
@@ -44,3 +44,18 @@
 
 #endif
 #endif
+
+#ifdef __ppc__
+__ibm128 i;
+template <> struct __is_floating_point_helper<__ibm128> {};
+int w(int x, __ibm128 *y) {
+  return x + *y;
+}
+// expected-no-error {{__ibm128 is not supported on this target}}
+#else
+__ibm128 i; // expected-error {{__ibm128 is not supported on this target}}
+template <> struct __is_floating_point_helper<__ibm128> {}; // expected-error {{__ibm128 is not supported on this target}}
+int w(int x, __ibm128 *y) { // expected-error {{__ibm128 is not supported on this target}}
+  return x + *y;
+}
+#endif
Index: clang/test/CodeGenCXX/ibm128-declarations.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ibm128-declarations.cpp
@@ -0,0 +1,169 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64-unknown-unknown \
+// RUN:   -std=c++20 %s -o - -debug-info-kind=limited | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown \
+// RUN:   -std=c++20 %s -o - -debug-info-kind=limited | FileCheck %s
+
+#include 
+
+static __ibm128 sgf;
+__ibm128 arrgf[10];
+__ibm128 func1(__ibm128 arg);
+
+class CTest {
+  __ibm128 pf;
+  static const __ibm128 scf;
+  volatile __ibm128 vf;
+
+public:
+  CTest(__ibm128 arg) : pf(arg), vf(arg) {}
+  __ibm128 func2(__ibm128 arg) {
+return pf + arg;
+  }
+  static __ibm128 func3(__ibm128 arg) {
+return arg * CTest::scf;
+  }
+};
+
+constexpr __ibm128 func_add(__ibm128 a, __ibm128 b) {
+  return a + b;
+}
+
+constinit const __ibm128 ci = func_add(1.0, 2.0);
+__ibm128 gf = ci;
+
+__ibm128 func_arith(__ibm128 a, __ibm128 b, __ibm128 c) {
+  __ibm128 v1 = a + b;
+  __ibm128 v2 = a - c;
+  __ibm128 v3 = v1 * c;
+  __ibm128 v4 = v2 / v3;
+  return v4;
+}
+
+__ibm128 

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Okay, thanks.  LGTM, then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5159
 BT->getKind() == BuiltinType::LongDouble ||
+BT->getKind() == BuiltinType::Ibm128 ||
 (getContext().getTargetInfo().hasFloat128Type() &&

rjmccall wrote:
> qiucf wrote:
> > rjmccall wrote:
> > > I hesitate to ask this, but does `__ibm128` form homogeneous aggregates 
> > > with `double`s?
> > > Homogeneous floating-point aggregates can have up to four IBM EXTENDED 
> > > PRECISION members, four IEEE BINARY 128 QUADRUPLE PRECISION members, four 
> > > _Decimal128 members, or eight members of other floating-point types. 
> > > (Unions are treated as their largest member. For homogeneous unions, 
> > > different union alternatives may have different sizes, provided that all 
> > > union members are homogeneous with respect to each other.) They are 
> > > passed in floating-point registers if parameters of that type would be 
> > > passed in floating-point registers. They are passed in vector registers 
> > > if parameters of that type would be passed in vector registers. They are 
> > > passed as if each member was specified as a separate parameter.
> > 
> > Yes.
> No, I mean, would an `__ibm128` in a struct with a pair of `double`s be 
> treated as if it were 4 `double`s, or is it considered non-homogeneous the 
> same way that a struct with e.g. 4 `float`s and 2 `double`s would be 
> non-homogeneous?
Ah, I think not. `__ibm128` (PowerPC `long double` also produces `ppc_fp128`) 
is distinct type in determining homogeneous aggregates.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 369045.
qiucf marked 3 inline comments as done.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

Files:
  clang/bindings/python/clang/cindex.py
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/CodeGen/ibm128-cast.c
  clang/test/CodeGen/ibm128-unsupported.c
  clang/test/CodeGenCXX/ibm128-declarations.cpp
  clang/test/Sema/128bitfloat.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -60,6 +60,7 @@
 BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
+BTCASE(Ibm128);
 BTCASE(NullPtr);
 BTCASE(Overload);
 BTCASE(Dependent);
@@ -577,6 +578,7 @@
 TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
+TKIND(Ibm128);
 TKIND(NullPtr);
 TKIND(Overload);
 TKIND(Dependent);
Index: clang/test/Sema/128bitfloat.cpp
===
--- clang/test/Sema/128bitfloat.cpp
+++ clang/test/Sema/128bitfloat.cpp
@@ -20,7 +20,7 @@
   return x + *y;
 }
 
-// expected-no-diagnostics
+// expected-no-error {{__float128 is not supported on this target}}
 #else
 #if !defined(__STRICT_ANSI__)
 __float128 f;  // expected-error {{__float128 is not supported on this target}}
@@ -44,3 +44,18 @@
 
 #endif
 #endif
+
+#ifdef __ppc__
+__ibm128 i;
+template <> struct __is_floating_point_helper<__ibm128> {};
+int w(int x, __ibm128 *y) {
+  return x + *y;
+}
+// expected-no-error {{__ibm128 is not supported on this target}}
+#else
+__ibm128 i; // expected-error {{__ibm128 is not supported on this target}}
+template <> struct __is_floating_point_helper<__ibm128> {}; // expected-error {{__ibm128 is not supported on this target}}
+int w(int x, __ibm128 *y) { // expected-error {{__ibm128 is not supported on this target}}
+  return x + *y;
+}
+#endif
Index: clang/test/CodeGenCXX/ibm128-declarations.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ibm128-declarations.cpp
@@ -0,0 +1,169 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64-unknown-unknown \
+// RUN:   -std=c++20 %s -o - -debug-info-kind=limited | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown \
+// RUN:   -std=c++20 %s -o - -debug-info-kind=limited | FileCheck %s
+
+#include 
+
+static __ibm128 sgf;
+__ibm128 arrgf[10];
+__ibm128 func1(__ibm128 arg);
+
+class CTest {
+  __ibm128 pf;
+  static const __ibm128 scf;
+  volatile __ibm128 vf;
+
+public:
+  CTest(__ibm128 arg) : pf(arg), vf(arg) {}
+  __ibm128 func2(__ibm128 arg) {
+return pf + arg;
+  }
+  static __ibm128 func3(__ibm128 arg) {
+return arg * CTest::scf;
+  }
+};
+
+constexpr __ibm128 func_add(__ibm128 a, __ibm128 b) {
+  return a + b;
+}
+
+constinit const __ibm128 ci = func_add(1.0, 2.0);
+__ibm128 gf = ci;
+
+__ibm128 func_arith(__ibm128 a, __ibm128 b, __ibm128 c) {
+  __ibm128 v1 = a + b;
+  __ibm128 v2 = a - c;
+  __ibm128 v3 = v1 * c;
+  __ibm128 v4 = v2 / v3;
+  return v4;
+}
+
+__ibm128 func_vaarg(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  __ibm128 r = va_arg(ap, __ibm128);
+  va_end(ap);
+  return r;
+}
+
+template  struct T1 {
+  T mem1;
+};
+template <> struct T1<__ibm128> {
+  __ibm128 mem2;
+};
+
+template 

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/include/clang/Basic/TargetInfo.h:131
+Float128,
+Ibm128
   };

qiucf wrote:
> rjmccall wrote:
> > This is necessary because it's possible to derive this type from a mode 
> > attribute?
> Yes, `__attribute__((mode(IF)))`
Okay.   I guess I agree that it makes sense to do that in a follow-up patch 
since it requires threading some extra state around.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5159
 BT->getKind() == BuiltinType::LongDouble ||
+BT->getKind() == BuiltinType::Ibm128 ||
 (getContext().getTargetInfo().hasFloat128Type() &&

qiucf wrote:
> rjmccall wrote:
> > I hesitate to ask this, but does `__ibm128` form homogeneous aggregates 
> > with `double`s?
> > Homogeneous floating-point aggregates can have up to four IBM EXTENDED 
> > PRECISION members, four IEEE BINARY 128 QUADRUPLE PRECISION members, four 
> > _Decimal128 members, or eight members of other floating-point types. 
> > (Unions are treated as their largest member. For homogeneous unions, 
> > different union alternatives may have different sizes, provided that all 
> > union members are homogeneous with respect to each other.) They are passed 
> > in floating-point registers if parameters of that type would be passed in 
> > floating-point registers. They are passed in vector registers if parameters 
> > of that type would be passed in vector registers. They are passed as if 
> > each member was specified as a separate parameter.
> 
> Yes.
No, I mean, would an `__ibm128` in a struct with a pair of `double`s be treated 
as if it were 4 `double`s, or is it considered non-homogeneous the same way 
that a struct with e.g. 4 `float`s and 2 `double`s would be non-homogeneous?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf marked 2 inline comments as done.
qiucf added inline comments.



Comment at: clang/include/clang/Basic/TargetInfo.h:131
+Float128,
+Ibm128
   };

rjmccall wrote:
> This is necessary because it's possible to derive this type from a mode 
> attribute?
Yes, `__attribute__((mode(IF)))`



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5159
 BT->getKind() == BuiltinType::LongDouble ||
+BT->getKind() == BuiltinType::Ibm128 ||
 (getContext().getTargetInfo().hasFloat128Type() &&

rjmccall wrote:
> I hesitate to ask this, but does `__ibm128` form homogeneous aggregates with 
> `double`s?
> Homogeneous floating-point aggregates can have up to four IBM EXTENDED 
> PRECISION members, four IEEE BINARY 128 QUADRUPLE PRECISION members, four 
> _Decimal128 members, or eight members of other floating-point types. (Unions 
> are treated as their largest member. For homogeneous unions, different union 
> alternatives may have different sizes, provided that all union members are 
> homogeneous with respect to each other.) They are passed in floating-point 
> registers if parameters of that type would be passed in floating-point 
> registers. They are passed in vector registers if parameters of that type 
> would be passed in vector registers. They are passed as if each member was 
> specified as a separate parameter.

Yes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

akyrtzi added inline comments.



Comment at: clang/lib/Index/USRGeneration.cpp:708
   c = 'd'; break;
+case BuiltinType::Ibm128: // FIXME: Need separate tag
 case BuiltinType::LongDouble:

rjmccall wrote:
> @akyrtzi @benlangmuir We can just add new USR codes for new types, right?
Yes, it would be good to do that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[John McCall via Phabricator via cfe-commits]

rjmccall added subscribers: akyrtzi, benlangmuir.
rjmccall added inline comments.



Comment at: clang/include/clang/Basic/TargetInfo.h:131
+Float128,
+Ibm128
   };

This is necessary because it's possible to derive this type from a mode 
attribute?



Comment at: clang/include/clang/Basic/TargetInfo.h:680
+  /// Return the mangled code of __ibm128.
+  virtual const char *getIbm128Mangling() const { return "g"; }
+

nemanjai wrote:
> The same mangling for both of these types seems incorrect as Steven pointed 
> out. Why not do something similar to what was done for `bfloat16` (i.e. an 
> `llvm_unreachable`) and override it for PowerPC?
Why are these manglings abstracted into TargetInfo in the first place?  Are 
there targets that are going to use a different mangling for `__ibm128`?



Comment at: clang/lib/AST/ASTContext.cpp:6274
+/// __float128 and __ibm128)
+bool ASTContext::areUnorderedFloatingTypes(QualType LHS, QualType RHS) const {
+  auto *LHSComplex = LHS->getAs();

Is it language semantics that these are unordered or just an implementation 
limitation?  I know WG14 is working on a draft for extended types that allows 
pairs to not be ordered, but IIRC that's supposed to be based on whether types 
have a subset relationship.  I believe the only FP formats that pairwise don't 
have subset relationships are `bfloat`/`half` and `ibm128`/`fp80`, and we don't 
support `ibm128` on any x86 targets so the latter is impossible in practice.  
The reason we have this restriction on converting between `ibm128` and 
`float128` is just that we haven't implemented the conversion in the backend / 
compiler-rt.  I don't think we should lock that limitation in as an actual 
unordered-type relationship.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5159
 BT->getKind() == BuiltinType::LongDouble ||
+BT->getKind() == BuiltinType::Ibm128 ||
 (getContext().getTargetInfo().hasFloat128Type() &&

I hesitate to ask this, but does `__ibm128` form homogeneous aggregates with 
`double`s?



Comment at: clang/lib/Index/USRGeneration.cpp:708
   c = 'd'; break;
+case BuiltinType::Ibm128: // FIXME: Need separate tag
 case BuiltinType::LongDouble:

@akyrtzi @benlangmuir We can just add new USR codes for new types, right?



Comment at: clang/lib/Sema/Sema.cpp:1842
+LongDoubleMismatched = true;
+}
+

I think this needs a comment explaining what you're checking for.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added a comment.

Ping.. Any further comments?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:2668
   // ::= g  # __float128
+  // ::= g  # __ibm128
   // UNSUPPORTED:::= Dd # IEEE 754r decimal floating point (64 bits)

jwakely wrote:
> steven.zhang wrote:
> > This is a bit confusing as the 'g' is for both __float128 and __ibm128. I 
> > know that PowerPC will mangle the __float128 as "u9__ieee128", and "g" for 
> > __ibm128. But c++filt demangle the "g" as __float128 which seems not right. 
> The (de)mangling is very confusing, but that's consistent with gcc.
> See comment 3 and 4 at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98389#c3
> 
> I think double-double should never have used `g` it should have been 
> something else (e.g. `u8__ibm128`), and then we could have `g` for the 
> `__ieee128` (aka `__float128`) type. But it is many years too late to change 
> that now.
Yeah, I think that's an unfortunate but ultimately understandable platform 
decision.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:2668
   // ::= g  # __float128
+  // ::= g  # __ibm128
   // UNSUPPORTED:::= Dd # IEEE 754r decimal floating point (64 bits)

steven.zhang wrote:
> This is a bit confusing as the 'g' is for both __float128 and __ibm128. I 
> know that PowerPC will mangle the __float128 as "u9__ieee128", and "g" for 
> __ibm128. But c++filt demangle the "g" as __float128 which seems not right. 
The (de)mangling is very confusing, but that's consistent with gcc.
See comment 3 and 4 at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98389#c3

I think double-double should never have used `g` it should have been something 
else (e.g. `u8__ibm128`), and then we could have `g` for the `__ieee128` (aka 
`__float128`) type. But it is many years too late to change that now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added inline comments.



Comment at: clang/bindings/python/clang/cindex.py:2061-2062
 TypeKind.FLOAT128 = TypeKind(30)
 TypeKind.HALF = TypeKind(31)
+TypeKind.IBM128 = TypeKind(32)
 TypeKind.COMPLEX = TypeKind(100)

hubert.reinterpretcast wrote:
> This looks suspiciously like the result of this file having not been 
> maintained for the additions of:
> ```
>   CXType_Float16 = 32,
>   CXType_ShortAccum = 33,
>   CXType_Accum = 34,
>   CXType_LongAccum = 35,
>   CXType_UShortAccum = 36,
>   CXType_UAccum = 37,
>   CXType_ULongAccum = 38,
>   CXType_BFloat16 = 39,
> ```
> 
> What test coverage fails if the addition of `TypeKind.IBM128` is omitted from 
> this patch?
Change this to `40` for consistency.

Actually I did not get any failure of `check-clang` even when removing all 
these floating point types assignment. Test coverage may miss here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 330560.
qiucf marked an inline comment as done.
qiucf added a comment.

Update cindex


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

Files:
  clang/bindings/python/clang/cindex.py
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/CodeGen/ibm128-cast.c
  clang/test/CodeGen/ibm128-unsupported.c
  clang/test/CodeGenCXX/ibm128-declarations.cpp
  clang/test/Sema/128bitfloat.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -60,6 +60,7 @@
 BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
+BTCASE(Ibm128);
 BTCASE(NullPtr);
 BTCASE(Overload);
 BTCASE(Dependent);
@@ -577,6 +578,7 @@
 TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
+TKIND(Ibm128);
 TKIND(NullPtr);
 TKIND(Overload);
 TKIND(Dependent);
Index: clang/test/Sema/128bitfloat.cpp
===
--- clang/test/Sema/128bitfloat.cpp
+++ clang/test/Sema/128bitfloat.cpp
@@ -13,7 +13,7 @@
   return x + *y;
 }
 
-// expected-no-diagnostics
+// expected-no-error {{__float128 is not supported on this target}}
 #else
 #if !defined(__STRICT_ANSI__)
 __float128 f;  // expected-error {{__float128 is not supported on this target}}
@@ -37,3 +37,18 @@
 
 #endif
 #endif
+
+#ifdef __ppc__
+__ibm128 i;
+template<> struct __is_floating_point_helper<__ibm128> {};
+int w(int x, __ibm128 *y) {
+  return x + *y;
+}
+// expected-no-error {{__ibm128 is not supported on this target}}
+#else
+__ibm128 i; // expected-error {{__ibm128 is not supported on this target}}
+template<> struct __is_floating_point_helper<__ibm128> {}; // expected-error {{__ibm128 is not supported on this target}}
+int w(int x, __ibm128 *y) { // expected-error {{__ibm128 is not supported on this target}}
+  return x + *y;
+}
+#endif
Index: clang/test/CodeGenCXX/ibm128-declarations.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ibm128-declarations.cpp
@@ -0,0 +1,168 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64-unknown-unknown \
+// RUN:   -std=c++20 %s -o - -debug-info-kind=limited | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown \
+// RUN:   -std=c++20 %s -o - -debug-info-kind=limited | FileCheck %s
+
+#include 
+
+static __ibm128 sgf;
+__ibm128 arrgf[10];
+__ibm128 func1(__ibm128 arg);
+
+class CTest {
+  __ibm128 pf;
+  static const __ibm128 scf;
+  volatile __ibm128 vf;
+public:
+  CTest(__ibm128 arg) : pf(arg), vf(arg) {}
+  __ibm128 func2(__ibm128 arg) {
+return pf + arg;
+  }
+  static __ibm128 func3(__ibm128 arg) {
+return arg * CTest::scf;
+  }
+};
+
+constexpr __ibm128 func_add(__ibm128 a, __ibm128 b) {
+  return a + b;
+}
+
+constinit const __ibm128 ci = func_add(1.0, 2.0);
+__ibm128 gf = ci;
+
+__ibm128 func_arith(__ibm128 a, __ibm128 b, __ibm128 c) {
+  __ibm128 v1 = a + b;
+  __ibm128 v2 = a - c;
+  __ibm128 v3 = v1 * c;
+  __ibm128 v4 = v2 / v3;
+  return v4;
+}
+
+__ibm128 func_vaarg(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  __ibm128 r = va_arg(ap, __ibm128);
+  va_end(ap);
+  return r;
+}
+
+template  struct T1 {
+  T mem1;
+};
+template <> struct T1<__ibm128> {
+  __ibm128 mem2;
+};
+
+template <__ibm128 Q> struct T2 {
+  constexpr static 

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/bindings/python/clang/cindex.py:2061-2062
 TypeKind.FLOAT128 = TypeKind(30)
 TypeKind.HALF = TypeKind(31)
+TypeKind.IBM128 = TypeKind(32)
 TypeKind.COMPLEX = TypeKind(100)

This looks suspiciously like the result of this file having not been maintained 
for the additions of:
```
  CXType_Float16 = 32,
  CXType_ShortAccum = 33,
  CXType_Accum = 34,
  CXType_LongAccum = 35,
  CXType_UShortAccum = 36,
  CXType_UAccum = 37,
  CXType_ULongAccum = 38,
  CXType_BFloat16 = 39,
```

What test coverage fails if the addition of `TypeKind.IBM128` is omitted from 
this patch?



Comment at: clang/include/clang-c/Index.h:3283
   CXType_FirstBuiltin = CXType_Void,
   CXType_LastBuiltin = CXType_BFloat16,
 

Looks like `CXType_Ibm128` is the "last" built-in type now?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

jwakely wrote:
> hubert.reinterpretcast wrote:
> > jwakely wrote:
> > > hubert.reinterpretcast wrote:
> > > > qiucf wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > qiucf wrote:
> > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > Not sure what the best method is to implement this, but `long 
> > > > > > > > double` and `__ibm128` are the same type for GCC when 
> > > > > > > > `-mabi=ibmlongdouble` is in effect.
> > > > > > > Seems clang is also different from GCC under 
> > > > > > > `-mabi=ieeelongdouble`? I saw `__float128` and `long double` are 
> > > > > > > the same for GCC but not for clang.
> > > > > > Have you checked whether the new libstdc++ for which this support 
> > > > > > is being added needs the GCC behaviour to work properly?
> > > > > > 
> > > > > > The GCC behaviour allows the following to be compiled without 
> > > > > > introducing novel overload resolution tiebreakers:
> > > > > > ```
> > > > > > void f(__float128);
> > > > > > void f(__ibm128);
> > > > > > void f(int);
> > > > > > 
> > > > > > long double ld;
> > > > > > 
> > > > > > int main() { f(ld); }
> > > > > > ```
> > > > > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > > > > 
> > > > > ```
> > > > > std::cout << "long double:\n";
> > > > > std::cout << std::numeric_limits::max() << std::endl;
> > > > > std::cout << std::numeric_limits::min() << std::endl;
> > > > > 
> > > > > std::cout << "__float128:\n";
> > > > > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > > > > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > > > > 
> > > > > std::cout << "__ibm128:\n";
> > > > > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > > > > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > > > > ```
> > > > @jwakely, are the overload resolution errors expected? @qiucf, are you 
> > > > sure you have a sufficiently new libstdc++?
> > > > @jwakely, are the overload resolution errors expected? 
> > > 
> > > Yes. Iostreams support `long double` but not `__float128`, unless that 
> > > happens to be the same type as `long double` (due to a 
> > > `-mabi=ieeelongdouble` option).
> > Meaning that Clang's `__float128` iosteams support (with libstdc++) would 
> > diverge from GCC.
> > 
> > For example, Clang reports the call below as ambiguous even with 
> > `-mabi=ieeelongdouble`:
> > ```
> > void f(double);
> > void f(long double);
> > 
> > void g(__float128 f128) { f(f128); }
> > ```
> > 
> > https://godbolt.org/z/dhYEKa
> > 
> > Insofar as the user experience goes, is this lack of iostreams support for 
> > `__float128` (even with `-mabi=ieeelongdouble`) within the realm of the 
> > intended design of libstdc++?
> The lack of iostreams support for `__float128` is the intended design.
> 
> On power we support `float`, `double` and three types of `long double`. If 
> `__float128` is a distinct type from all those `long double` types it won't 
> work.
> 
> GCC on power defines `__float128` as a macro expanding to `__ieee128`, so it 
> is the same as one of the `long double` types.
Okay, it sounds like the different treatment Clang has does not really 
interfere with recommended usage insofar as libstdc++ iostreams (and hopefully 
this extends to the rest of the library).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

hubert.reinterpretcast wrote:
> jwakely wrote:
> > hubert.reinterpretcast wrote:
> > > qiucf wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > qiucf wrote:
> > > > > > hubert.reinterpretcast wrote:
> > > > > > > Not sure what the best method is to implement this, but `long 
> > > > > > > double` and `__ibm128` are the same type for GCC when 
> > > > > > > `-mabi=ibmlongdouble` is in effect.
> > > > > > Seems clang is also different from GCC under 
> > > > > > `-mabi=ieeelongdouble`? I saw `__float128` and `long double` are 
> > > > > > the same for GCC but not for clang.
> > > > > Have you checked whether the new libstdc++ for which this support is 
> > > > > being added needs the GCC behaviour to work properly?
> > > > > 
> > > > > The GCC behaviour allows the following to be compiled without 
> > > > > introducing novel overload resolution tiebreakers:
> > > > > ```
> > > > > void f(__float128);
> > > > > void f(__ibm128);
> > > > > void f(int);
> > > > > 
> > > > > long double ld;
> > > > > 
> > > > > int main() { f(ld); }
> > > > > ```
> > > > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > > > 
> > > > ```
> > > > std::cout << "long double:\n";
> > > > std::cout << std::numeric_limits::max() << std::endl;
> > > > std::cout << std::numeric_limits::min() << std::endl;
> > > > 
> > > > std::cout << "__float128:\n";
> > > > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > > > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > > > 
> > > > std::cout << "__ibm128:\n";
> > > > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > > > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > > > ```
> > > @jwakely, are the overload resolution errors expected? @qiucf, are you 
> > > sure you have a sufficiently new libstdc++?
> > > @jwakely, are the overload resolution errors expected? 
> > 
> > Yes. Iostreams support `long double` but not `__float128`, unless that 
> > happens to be the same type as `long double` (due to a 
> > `-mabi=ieeelongdouble` option).
> Meaning that Clang's `__float128` iosteams support (with libstdc++) would 
> diverge from GCC.
> 
> For example, Clang reports the call below as ambiguous even with 
> `-mabi=ieeelongdouble`:
> ```
> void f(double);
> void f(long double);
> 
> void g(__float128 f128) { f(f128); }
> ```
> 
> https://godbolt.org/z/dhYEKa
> 
> Insofar as the user experience goes, is this lack of iostreams support for 
> `__float128` (even with `-mabi=ieeelongdouble`) within the realm of the 
> intended design of libstdc++?
The lack of iostreams support for `__float128` is the intended design.

On power we support `float`, `double` and three types of `long double`. If 
`__float128` is a distinct type from all those `long double` types it won't 
work.

GCC on power defines `__float128` as a macro expanding to `__ieee128`, so it is 
the same as one of the `long double` types.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

jwakely wrote:
> hubert.reinterpretcast wrote:
> > qiucf wrote:
> > > hubert.reinterpretcast wrote:
> > > > qiucf wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > Not sure what the best method is to implement this, but `long 
> > > > > > double` and `__ibm128` are the same type for GCC when 
> > > > > > `-mabi=ibmlongdouble` is in effect.
> > > > > Seems clang is also different from GCC under `-mabi=ieeelongdouble`? 
> > > > > I saw `__float128` and `long double` are the same for GCC but not for 
> > > > > clang.
> > > > Have you checked whether the new libstdc++ for which this support is 
> > > > being added needs the GCC behaviour to work properly?
> > > > 
> > > > The GCC behaviour allows the following to be compiled without 
> > > > introducing novel overload resolution tiebreakers:
> > > > ```
> > > > void f(__float128);
> > > > void f(__ibm128);
> > > > void f(int);
> > > > 
> > > > long double ld;
> > > > 
> > > > int main() { f(ld); }
> > > > ```
> > > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > > 
> > > ```
> > > std::cout << "long double:\n";
> > > std::cout << std::numeric_limits::max() << std::endl;
> > > std::cout << std::numeric_limits::min() << std::endl;
> > > 
> > > std::cout << "__float128:\n";
> > > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > > 
> > > std::cout << "__ibm128:\n";
> > > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > > ```
> > @jwakely, are the overload resolution errors expected? @qiucf, are you sure 
> > you have a sufficiently new libstdc++?
> > @jwakely, are the overload resolution errors expected? 
> 
> Yes. Iostreams support `long double` but not `__float128`, unless that 
> happens to be the same type as `long double` (due to a `-mabi=ieeelongdouble` 
> option).
Meaning that Clang's `__float128` iosteams support (with libstdc++) would 
diverge from GCC.

For example, Clang reports the call below as ambiguous even with 
`-mabi=ieeelongdouble`:
```
void f(double);
void f(long double);

void g(__float128 f128) { f(f128); }
```

https://godbolt.org/z/dhYEKa

Insofar as the user experience goes, is this lack of iostreams support for 
`__float128` (even with `-mabi=ieeelongdouble`) within the realm of the 
intended design of libstdc++?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

hubert.reinterpretcast wrote:
> qiucf wrote:
> > hubert.reinterpretcast wrote:
> > > qiucf wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > Not sure what the best method is to implement this, but `long double` 
> > > > > and `__ibm128` are the same type for GCC when `-mabi=ibmlongdouble` 
> > > > > is in effect.
> > > > Seems clang is also different from GCC under `-mabi=ieeelongdouble`? I 
> > > > saw `__float128` and `long double` are the same for GCC but not for 
> > > > clang.
> > > Have you checked whether the new libstdc++ for which this support is 
> > > being added needs the GCC behaviour to work properly?
> > > 
> > > The GCC behaviour allows the following to be compiled without introducing 
> > > novel overload resolution tiebreakers:
> > > ```
> > > void f(__float128);
> > > void f(__ibm128);
> > > void f(int);
> > > 
> > > long double ld;
> > > 
> > > int main() { f(ld); }
> > > ```
> > As I saw both GCC and clang have error for ambiguous `operator<<` for:
> > 
> > ```
> > std::cout << "long double:\n";
> > std::cout << std::numeric_limits::max() << std::endl;
> > std::cout << std::numeric_limits::min() << std::endl;
> > 
> > std::cout << "__float128:\n";
> > std::cout << std::numeric_limits<__float128>::max() << std::endl;
> > std::cout << std::numeric_limits<__float128>::min() << std::endl;
> > 
> > std::cout << "__ibm128:\n";
> > std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> > std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> > ```
> @jwakely, are the overload resolution errors expected? @qiucf, are you sure 
> you have a sufficiently new libstdc++?
> @jwakely, are the overload resolution errors expected? 

Yes. Iostreams support `long double` but not `__float128`, unless that happens 
to be the same type as `long double` (due to a `-mabi=ieeelongdouble` option).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

qiucf wrote:
> hubert.reinterpretcast wrote:
> > qiucf wrote:
> > > hubert.reinterpretcast wrote:
> > > > Not sure what the best method is to implement this, but `long double` 
> > > > and `__ibm128` are the same type for GCC when `-mabi=ibmlongdouble` is 
> > > > in effect.
> > > Seems clang is also different from GCC under `-mabi=ieeelongdouble`? I 
> > > saw `__float128` and `long double` are the same for GCC but not for clang.
> > Have you checked whether the new libstdc++ for which this support is being 
> > added needs the GCC behaviour to work properly?
> > 
> > The GCC behaviour allows the following to be compiled without introducing 
> > novel overload resolution tiebreakers:
> > ```
> > void f(__float128);
> > void f(__ibm128);
> > void f(int);
> > 
> > long double ld;
> > 
> > int main() { f(ld); }
> > ```
> As I saw both GCC and clang have error for ambiguous `operator<<` for:
> 
> ```
> std::cout << "long double:\n";
> std::cout << std::numeric_limits::max() << std::endl;
> std::cout << std::numeric_limits::min() << std::endl;
> 
> std::cout << "__float128:\n";
> std::cout << std::numeric_limits<__float128>::max() << std::endl;
> std::cout << std::numeric_limits<__float128>::min() << std::endl;
> 
> std::cout << "__ibm128:\n";
> std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
> std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
> ```
@jwakely, are the overload resolution errors expected? @qiucf, are you sure you 
have a sufficiently new libstdc++?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

hubert.reinterpretcast wrote:
> qiucf wrote:
> > hubert.reinterpretcast wrote:
> > > Not sure what the best method is to implement this, but `long double` and 
> > > `__ibm128` are the same type for GCC when `-mabi=ibmlongdouble` is in 
> > > effect.
> > Seems clang is also different from GCC under `-mabi=ieeelongdouble`? I saw 
> > `__float128` and `long double` are the same for GCC but not for clang.
> Have you checked whether the new libstdc++ for which this support is being 
> added needs the GCC behaviour to work properly?
> 
> The GCC behaviour allows the following to be compiled without introducing 
> novel overload resolution tiebreakers:
> ```
> void f(__float128);
> void f(__ibm128);
> void f(int);
> 
> long double ld;
> 
> int main() { f(ld); }
> ```
As I saw both GCC and clang have error for ambiguous `operator<<` for:

```
std::cout << "long double:\n";
std::cout << std::numeric_limits::max() << std::endl;
std::cout << std::numeric_limits::min() << std::endl;

std::cout << "__float128:\n";
std::cout << std::numeric_limits<__float128>::max() << std::endl;
std::cout << std::numeric_limits<__float128>::min() << std::endl;

std::cout << "__ibm128:\n";
std::cout << std::numeric_limits<__ibm128>::max() << std::endl;
std::cout << std::numeric_limits<__ibm128>::min() << std::endl;
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 327716.
qiucf marked 2 inline comments as done.
qiucf added a comment.

- Add helper method to count 'unordered' cases
- Add missing tests (in last update)
- Add test for openmp device-host case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

Files:
  clang/bindings/python/clang/cindex.py
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/CodeGen/ibm128-cast.c
  clang/test/CodeGen/ibm128-unsupported.c
  clang/test/CodeGenCXX/ibm128-declarations.cpp
  clang/test/Sema/128bitfloat.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -60,6 +60,7 @@
 BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
+BTCASE(Ibm128);
 BTCASE(NullPtr);
 BTCASE(Overload);
 BTCASE(Dependent);
@@ -577,6 +578,7 @@
 TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
+TKIND(Ibm128);
 TKIND(NullPtr);
 TKIND(Overload);
 TKIND(Dependent);
Index: clang/test/Sema/128bitfloat.cpp
===
--- clang/test/Sema/128bitfloat.cpp
+++ clang/test/Sema/128bitfloat.cpp
@@ -13,7 +13,7 @@
   return x + *y;
 }
 
-// expected-no-diagnostics
+// expected-no-error {{__float128 is not supported on this target}}
 #else
 #if !defined(__STRICT_ANSI__)
 __float128 f;  // expected-error {{__float128 is not supported on this target}}
@@ -37,3 +37,18 @@
 
 #endif
 #endif
+
+#ifdef __ppc__
+__ibm128 i;
+template<> struct __is_floating_point_helper<__ibm128> {};
+int w(int x, __ibm128 *y) {
+  return x + *y;
+}
+// expected-no-error {{__ibm128 is not supported on this target}}
+#else
+__ibm128 i; // expected-error {{__ibm128 is not supported on this target}}
+template<> struct __is_floating_point_helper<__ibm128> {}; // expected-error {{__ibm128 is not supported on this target}}
+int w(int x, __ibm128 *y) { // expected-error {{__ibm128 is not supported on this target}}
+  return x + *y;
+}
+#endif
Index: clang/test/CodeGenCXX/ibm128-declarations.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ibm128-declarations.cpp
@@ -0,0 +1,168 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64-unknown-unknown \
+// RUN:   -std=c++20 %s -o - -debug-info-kind=limited | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown \
+// RUN:   -std=c++20 %s -o - -debug-info-kind=limited | FileCheck %s
+
+#include 
+
+static __ibm128 sgf;
+__ibm128 arrgf[10];
+__ibm128 func1(__ibm128 arg);
+
+class CTest {
+  __ibm128 pf;
+  static const __ibm128 scf;
+  volatile __ibm128 vf;
+public:
+  CTest(__ibm128 arg) : pf(arg), vf(arg) {}
+  __ibm128 func2(__ibm128 arg) {
+return pf + arg;
+  }
+  static __ibm128 func3(__ibm128 arg) {
+return arg * CTest::scf;
+  }
+};
+
+constexpr __ibm128 func_add(__ibm128 a, __ibm128 b) {
+  return a + b;
+}
+
+constinit const __ibm128 ci = func_add(1.0, 2.0);
+__ibm128 gf = ci;
+
+__ibm128 func_arith(__ibm128 a, __ibm128 b, __ibm128 c) {
+  __ibm128 v1 = a + b;
+  __ibm128 v2 = a - c;
+  __ibm128 v3 = v1 * c;
+  __ibm128 v4 = v2 / v3;
+  return v4;
+}
+
+__ibm128 func_vaarg(int n, ...) {
+  va_list ap;
+  va_start(ap, n);
+  __ibm128 r = va_arg(ap, __ibm128);
+  va_end(ap);
+  return r;
+}
+
+template  struct T1 {
+  T mem1;
+};

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D93377#2481957 , 
@hubert.reinterpretcast wrote:

> In D93377#2481312 , @rjmccall wrote:
>
>> Are you committed to the name `__ibm128`?
>
> Insofar as that's what GCC on Power (for example, `gcc (Ubuntu 
> 7.5.0-3ubuntu1~18.04) 7.5.0` from 2017) has shipped with for several years, 
> yes.

Okay, it wasn't clear from the description that this was an already-shipping 
type.  In that case, it's regrettable but obviously just something we have to 
match.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D93377#2481312 , @rjmccall wrote:

> Are you committed to the name `__ibm128`?

Insofar as that's what GCC on Power (for example, `gcc (Ubuntu 
7.5.0-3ubuntu1~18.04) 7.5.0` from 2017) has shipped with for several years, yes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Are you committed to the name `__ibm128`?  I think a name that says something 
about floating point would be better than just a company name and a number.  
"double double" is the common name for this format pretty much everywhere, and 
while I certainly would *not* suggest actually spelling it `double double` 
(i.e. with repeated type specifiers0), you could certainly use something like 
`__doubledouble`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:6230
 /// LHS < RHS, return -1.
 int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const {
   FloatingRank LHSR = getFloatingRank(LHS);

qiucf wrote:
> hubert.reinterpretcast wrote:
> > nemanjai wrote:
> > > hubert.reinterpretcast wrote:
> > > > I think this function should vocally fail when presented with 
> > > > "unordered" cases.
> > > But is it possible to emit errors here or should there be code explicitly 
> > > added to Sema to disallow conversions between `__ibm128` and `__float128` 
> > > (and `long double` to/from either of those to which it is not equivalent)?
> > I did not mean a user-facing error message. I meant that there should be 
> > some form of assertion or internal diagnostic here. I believe `assert` is 
> > appropriate.
> > 
> > I will note that this is a public member function of ASTContext. Regardless 
> > of explicit code in Sema that does what you describe, I think this function 
> > should not present an interface where it does not report "unordered" cases 
> > as unordered.
> > 
> Adding assertion here makes `unsupportedTypeConversion` always fail in such 
> cases.
Yes, I know. I think `unsupportedTypeConversion` should avoid calling this 
function when it is possibly dealing with an "unordered" case unless if this 
function has a way of indicating "unordered" as a result. If there is a helper 
function for testing the unordered case in this class, then I think 
`unsupportedTypeConversion` can simply say that all ordered cases are supported 
(before doing more to figure out the unordered cases that are safe).



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

qiucf wrote:
> hubert.reinterpretcast wrote:
> > Not sure what the best method is to implement this, but `long double` and 
> > `__ibm128` are the same type for GCC when `-mabi=ibmlongdouble` is in 
> > effect.
> Seems clang is also different from GCC under `-mabi=ieeelongdouble`? I saw 
> `__float128` and `long double` are the same for GCC but not for clang.
Have you checked whether the new libstdc++ for which this support is being 
added needs the GCC behaviour to work properly?

The GCC behaviour allows the following to be compiled without introducing novel 
overload resolution tiebreakers:
```
void f(__float128);
void f(__ibm128);
void f(int);

long double ld;

int main() { f(ld); }
```



Comment at: clang/lib/Sema/SemaType.cpp:1562-1563
+if (!S.Context.getTargetInfo().hasIbm128Type() &&
+!S.getLangOpts().SYCLIsDevice &&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported) << 
"__ibm128";

qiucf wrote:
> hubert.reinterpretcast wrote:
> > Do the SYCL and OpenMP device exceptions to the error really apply for 
> > `__ibm128`?
> If host supports `__ibm128` but device does not?
Can you add a test that makes use of this? Also, there should be a case that 
triggers `err_device_unsupported_type`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:6204
 case HalfRank: llvm_unreachable("Complex half is not supported");
+case Ibm128Rank: llvm_unreachable("Complex __ibm128 is not supported");
 case FloatRank:  return FloatComplexTy;

nemanjai wrote:
> Why? Is this coming in an upcoming patch or is this something that will never 
> be available?
Sure, I plan to add it in next patch



Comment at: clang/lib/AST/ASTContext.cpp:6230
 /// LHS < RHS, return -1.
 int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const {
   FloatingRank LHSR = getFloatingRank(LHS);

hubert.reinterpretcast wrote:
> nemanjai wrote:
> > hubert.reinterpretcast wrote:
> > > I think this function should vocally fail when presented with "unordered" 
> > > cases.
> > But is it possible to emit errors here or should there be code explicitly 
> > added to Sema to disallow conversions between `__ibm128` and `__float128` 
> > (and `long double` to/from either of those to which it is not equivalent)?
> I did not mean a user-facing error message. I meant that there should be some 
> form of assertion or internal diagnostic here. I believe `assert` is 
> appropriate.
> 
> I will note that this is a public member function of ASTContext. Regardless 
> of explicit code in Sema that does what you describe, I think this function 
> should not present an interface where it does not report "unordered" cases as 
> unordered.
> 
Adding assertion here makes `unsupportedTypeConversion` always fail in such 
cases.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

hubert.reinterpretcast wrote:
> Not sure what the best method is to implement this, but `long double` and 
> `__ibm128` are the same type for GCC when `-mabi=ibmlongdouble` is in effect.
Seems clang is also different from GCC under `-mabi=ieeelongdouble`? I saw 
`__float128` and `long double` are the same for GCC but not for clang.



Comment at: clang/lib/Sema/SemaExpr.cpp:1229
+   ::APFloat::PPCDoubleDouble())) ||
+ EitherIbm128;
 }

nemanjai wrote:
> hubert.reinterpretcast wrote:
> > Same comment about possible blocking of `double` to `__ibm128` conversions.
> I am not sure what "same comment" refers to, but I agree that this seems very 
> wrong - doesn't this mean that we can't convert between `__ibm128` and any 
> other floating point type?
> In any case, I think this can be represented quite concisely as something 
> like:
> `if ( && )`
I found this would break existing tests of x86_64: `x86_fp80` is 96 bits long 
on x86_32, 128 bits long on x86_64. However tests expect `x86_fp80` can be 
`fpext`ed into `fp128`.



Comment at: clang/lib/Sema/SemaType.cpp:1562-1563
+if (!S.Context.getTargetInfo().hasIbm128Type() &&
+!S.getLangOpts().SYCLIsDevice &&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported) << 
"__ibm128";

hubert.reinterpretcast wrote:
> Do the SYCL and OpenMP device exceptions to the error really apply for 
> `__ibm128`?
If host supports `__ibm128` but device does not?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 313835.
qiucf marked 14 inline comments as done.
qiucf added a comment.

Address comments and add tests


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

Files:
  clang/bindings/python/clang/cindex.py
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/Sema/128bitfloat.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -60,6 +60,7 @@
 BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
+BTCASE(Ibm128);
 BTCASE(NullPtr);
 BTCASE(Overload);
 BTCASE(Dependent);
@@ -577,6 +578,7 @@
 TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
+TKIND(Ibm128);
 TKIND(NullPtr);
 TKIND(Overload);
 TKIND(Dependent);
Index: clang/test/Sema/128bitfloat.cpp
===
--- clang/test/Sema/128bitfloat.cpp
+++ clang/test/Sema/128bitfloat.cpp
@@ -13,7 +13,7 @@
   return x + *y;
 }
 
-// expected-no-diagnostics
+// expected-no-error {{__float128 is not supported on this target}}
 #else
 #if !defined(__STRICT_ANSI__)
 __float128 f;  // expected-error {{__float128 is not supported on this target}}
@@ -37,3 +37,18 @@
 
 #endif
 #endif
+
+#ifdef __ppc__
+__ibm128 i;
+template<> struct __is_floating_point_helper<__ibm128> {};
+int w(int x, __ibm128 *y) {
+  return x + *y;
+}
+// expected-no-error {{__ibm128 is not supported on this target}}
+#else
+__ibm128 i; // expected-error {{__ibm128 is not supported on this target}}
+template<> struct __is_floating_point_helper<__ibm128> {}; // expected-error {{__ibm128 is not supported on this target}}
+int w(int x, __ibm128 *y) { // expected-error {{__ibm128 is not supported on this target}}
+  return x + *y;
+}
+#endif
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -6935,6 +6935,9 @@
 case PREDEF_TYPE_FLOAT128_ID:
   T = Context.Float128Ty;
   break;
+case PREDEF_TYPE_IBM128_ID:
+  T = Context.Ibm128Ty;
+  break;
 case PREDEF_TYPE_OVERLOAD_ID:
   T = Context.OverloadTy;
   break;
Index: clang/lib/Serialization/ASTCommon.cpp
===
--- clang/lib/Serialization/ASTCommon.cpp
+++ clang/lib/Serialization/ASTCommon.cpp
@@ -168,6 +168,9 @@
   case BuiltinType::Float128:
 ID = PREDEF_TYPE_FLOAT128_ID;
 break;
+  case BuiltinType::Ibm128:
+ID = PREDEF_TYPE_IBM128_ID;
+break;
   case BuiltinType::NullPtr:
 ID = PREDEF_TYPE_NULLPTR_ID;
 break;
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1557,6 +1557,13 @@
 << "__float128";
 Result = Context.Float128Ty;
 break;
+  case DeclSpec::TST_ibm128:
+if (!S.Context.getTargetInfo().hasIbm128Type() &&
+!S.getLangOpts().SYCLIsDevice &&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported) << "__ibm128";
+Result = Context.Ibm128Ty;
+break;
   case DeclSpec::TST_bool:
 Result = Context.BoolTy; // _Bool or bool
 break;
Index: clang/lib/Sema/SemaTemplateVariadic.cpp
===
--- 

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:6230
 /// LHS < RHS, return -1.
 int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const {
   FloatingRank LHSR = getFloatingRank(LHS);

nemanjai wrote:
> hubert.reinterpretcast wrote:
> > I think this function should vocally fail when presented with "unordered" 
> > cases.
> But is it possible to emit errors here or should there be code explicitly 
> added to Sema to disallow conversions between `__ibm128` and `__float128` 
> (and `long double` to/from either of those to which it is not equivalent)?
I did not mean a user-facing error message. I meant that there should be some 
form of assertion or internal diagnostic here. I believe `assert` is 
appropriate.

I will note that this is a public member function of ASTContext. Regardless of 
explicit code in Sema that does what you describe, I think this function should 
not present an interface where it does not report "unordered" cases as 
unordered.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

Seems that conversion diagnostic test cases are completely missing.




Comment at: clang/include/clang/Basic/TargetInfo.h:680
+  /// Return the mangled code of __ibm128.
+  virtual const char *getIbm128Mangling() const { return "g"; }
+

The same mangling for both of these types seems incorrect as Steven pointed 
out. Why not do something similar to what was done for `bfloat16` (i.e. an 
`llvm_unreachable`) and override it for PowerPC?



Comment at: clang/lib/AST/ASTContext.cpp:6204
 case HalfRank: llvm_unreachable("Complex half is not supported");
+case Ibm128Rank: llvm_unreachable("Complex __ibm128 is not supported");
 case FloatRank:  return FloatComplexTy;

Why? Is this coming in an upcoming patch or is this something that will never 
be available?



Comment at: clang/lib/AST/ASTContext.cpp:6230
 /// LHS < RHS, return -1.
 int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const {
   FloatingRank LHSR = getFloatingRank(LHS);

hubert.reinterpretcast wrote:
> I think this function should vocally fail when presented with "unordered" 
> cases.
But is it possible to emit errors here or should there be code explicitly added 
to Sema to disallow conversions between `__ibm128` and `__float128` (and `long 
double` to/from either of those to which it is not equivalent)?



Comment at: clang/lib/Sema/SemaExpr.cpp:1229
+   ::APFloat::PPCDoubleDouble())) ||
+ EitherIbm128;
 }

hubert.reinterpretcast wrote:
> Same comment about possible blocking of `double` to `__ibm128` conversions.
I am not sure what "same comment" refers to, but I agree that this seems very 
wrong - doesn't this mean that we can't convert between `__ibm128` and any 
other floating point type?
In any case, I think this can be represented quite concisely as something like:
`if ( && )`



Comment at: clang/lib/Sema/SemaOverload.cpp:1887
+  FromType == S.Context.Ibm128Ty || ToType == S.Context.Ibm128Ty;
+  if ((Float128AndLongDouble &&
+   ((S.Context.LongDoubleTy) ==

hubert.reinterpretcast wrote:
> This seems to disable conversion from `double` to `__ibm128`?
Oh, now I understand the "same comment" comment above. Yes, this appears to 
also be a case where we don't want to allow types of the same width but 
different representation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:25
+
+__ibm128 func4(__ibm128 a, __ibm128 b) {
+  return a + b;

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > There's a lot of missing testing, especially around implicit conversions, 
> > narrowing conversions, and the usual arithmetic conversions.
> > 
> Make this a `constexpr` function and call it from the initializer of a global 
> declared with `consteval`.
> Make this a `constexpr` function and call it from the initializer of a global 
> declared with `consteval`.

Sorry, I meant `constinit` and not `consteval`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:6230
 /// LHS < RHS, return -1.
 int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const {
   FloatingRank LHSR = getFloatingRank(LHS);

I think this function should vocally fail when presented with "unordered" cases.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:803
+  case BuiltinType::Ibm128:
 // FIXME: For targets where long double and __float128 have the same size,
 // they are currently indistinguishable in the debugger without some

Update the comment for `__ibm128`.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:2245
+  case tok::kw___ibm128:
+DS.SetTypeSpecType(DeclSpec::TST_ibm128, Loc, PrevSpec, DiagID, Policy);
+break;

Not sure what the best method is to implement this, but `long double` and 
`__ibm128` are the same type for GCC when `-mabi=ibmlongdouble` is in effect.



Comment at: clang/lib/Sema/SemaExpr.cpp:1188-1190
 /// Diagnose attempts to convert between __float128 and long double if
 /// there is no support for such conversion. Helper function of
 /// UsualArithmeticConversions().

Comment needs update.



Comment at: clang/lib/Sema/SemaExpr.cpp:1229
+   ::APFloat::PPCDoubleDouble())) ||
+ EitherIbm128;
 }

Same comment about possible blocking of `double` to `__ibm128` conversions.



Comment at: clang/lib/Sema/SemaExpr.cpp:1546-1547
 
   // Diagnose attempts to convert between __float128 and long double where
   // such conversions currently can't be handled.
   if (unsupportedTypeConversion(*this, LHSType, RHSType))

Comment needs update.



Comment at: clang/lib/Sema/SemaOverload.cpp:1887
+  FromType == S.Context.Ibm128Ty || ToType == S.Context.Ibm128Ty;
+  if ((Float128AndLongDouble &&
+   ((S.Context.LongDoubleTy) ==

This seems to disable conversion from `double` to `__ibm128`?



Comment at: clang/lib/Sema/SemaType.cpp:1562-1563
+if (!S.Context.getTargetInfo().hasIbm128Type() &&
+!S.getLangOpts().SYCLIsDevice &&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice))
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported) << 
"__ibm128";

Do the SYCL and OpenMP device exceptions to the error really apply for 
`__ibm128`?



Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:6
+
+__ibm128 gf;
+static __ibm128 sgf;

What does the debug info look like?



Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:24
+};
+
+__ibm128 func4(__ibm128 a, __ibm128 b) {

Add a function that reads an `__ibm128` from varargs.



Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:25
+
+__ibm128 func4(__ibm128 a, __ibm128 b) {
+  return a + b;

There's a lot of missing testing, especially around implicit conversions, 
narrowing conversions, and the usual arithmetic conversions.




Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:25
+
+__ibm128 func4(__ibm128 a, __ibm128 b) {
+  return a + b;

hubert.reinterpretcast wrote:
> There's a lot of missing testing, especially around implicit conversions, 
> narrowing conversions, and the usual arithmetic conversions.
> 
Make this a `constexpr` function and call it from the initializer of a global 
declared with `consteval`.



Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:29
+
+template  struct T1 {
+  T mem1;

Add a case where `__ibm128` is used as the type of a non-type template 
parameter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qing Shan Zhang via Phabricator via cfe-commits]

steven.zhang added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:2668
   // ::= g  # __float128
+  // ::= g  # __ibm128
   // UNSUPPORTED:::= Dd # IEEE 754r decimal floating point (64 bits)

This is a bit confusing as the 'g' is for both __float128 and __ibm128. I know 
that PowerPC will mangle the __float128 as "u9__ieee128", and "g" for __ibm128. 
But c++filt demangle the "g" as __float128 which seems not right. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93377/new/

https://reviews.llvm.org/D93377

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf created this revision.
qiucf added reviewers: nemanjai, rjmccall, rsmith, hubert.reinterpretcast, 
akyrtzi, PowerPC.
Herald added subscribers: dexonsmith, steven.zhang, shchenz, arphaman, kbarton.
qiucf requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Currently, we have no front-end type for `ppc_fp128` type in IR. PowerPC target 
generates `ppc_fp128` type from `long double` now, but there's option 
(`-mabi=(ieee|ibm)longdouble`) to control it and we're going to do transition 
from IBM extended double-double `ppc_fp128` to IEEE `fp128` in the future.

This patch adds type `__ibm128` which always represents `ppc_fp128` in IR, as 
what GCC did for that type. Without this type in Clang, compilation will fail 
if compiling against future version of libstdcxx (which uses `__ibm128` in 
headers).

Although all operations in backend for `__ibm128` is done by software, only 
PowerPC enables support for it.

There's something not implemented in this patch and can be done in future ones:

1. Literal suffix for `__ibm128` type. `w/W` is suitable as GCC documented 
(https://gcc.gnu.org/onlinedocs/gcc/Floating-Types.html) but I believe they 
haven't implemented it yet.
2. `__attribute__((mode(IF)))` should be for `__ibm128`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D93377

Files:
  clang/bindings/python/clang/cindex.py
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/CodeGenCXX/ibm128-declarations.cpp
  clang/test/Sema/128bitfloat.cpp
  clang/tools/libclang/CXType.cpp

Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -60,6 +60,7 @@
 BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
+BTCASE(Ibm128);
 BTCASE(NullPtr);
 BTCASE(Overload);
 BTCASE(Dependent);
@@ -577,6 +578,7 @@
 TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
+TKIND(Ibm128);
 TKIND(NullPtr);
 TKIND(Overload);
 TKIND(Dependent);
Index: clang/test/Sema/128bitfloat.cpp
===
--- clang/test/Sema/128bitfloat.cpp
+++ clang/test/Sema/128bitfloat.cpp
@@ -13,7 +13,7 @@
   return x + *y;
 }
 
-// expected-no-diagnostics
+// expected-no-error {{__float128 is not supported on this target}}
 #else
 #if !defined(__STRICT_ANSI__)
 __float128 f;  // expected-error {{__float128 is not supported on this target}}
@@ -37,3 +37,18 @@
 
 #endif
 #endif
+
+#ifdef __ppc__
+__ibm128 i;
+template<> struct __is_floating_point_helper<__ibm128> {};
+int w(int x, __ibm128 *y) {
+  return x + *y;
+}
+// expected-no-error {{__ibm128 is not supported on this target}}
+#else
+__ibm128 i; // expected-error {{__ibm128 is not supported on this target}}
+template<> struct __is_floating_point_helper<__ibm128> {}; // expected-error {{__ibm128 is not supported on this target}}
+int w(int x, __ibm128 *y) { // expected-error {{__ibm128 is not supported on this target}}
+  return x + *y;
+}
+#endif
Index: clang/test/CodeGenCXX/ibm128-declarations.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/ibm128-declarations.cpp
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64-unknown-unknown \
+// RUN:   -std=c++11 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple powerpc64le-unknown-unknown \
+// RUN:   -std=c++11 %s -o - | FileCheck %s
+
+__ibm128 gf;
+static __ibm128 sgf;
+__ibm128 arrgf[10];
+__ibm128 func1(__ibm128 arg);
+
+class CTest {
+