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
+ &llvm::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 (<types have same size> && <types do not have same fltSemantics>)`
================
Comment at: clang/lib/Sema/SemaOverload.cpp:1887
+ FromType == S.Context.Ibm128Ty || ToType == S.Context.Ibm128Ty;
+ if ((Float128AndLongDouble &&
+ (&S.Context.getFloatTypeSemantics(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
