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<ComplexType>();
----------------
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
