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
