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
+ &llvm::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 (<types have same size> && <types do not have same fltSemantics>)`
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
