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
+ &llvm::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.getFloatTypeSemantics(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 <typename T> 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
