codemzs added inline comments.
================
Comment at: clang/lib/Sema/SemaCast.cpp:1367
+ Self.Context.doCXX23ExtendedFpTypesRulesApply(DestType, SrcType)) {
+ // Support for cast between fp16 and bf16 doesn't exist yet.
+ if (!((DestType->isBFloat16Type() || DestType->isFloat16Type()) &&
----------------
tahonermann wrote:
> Should this be a FIXME comment? What happens in this case? Should we perhaps
> assert on such attempted conversions?
I have added the FIXME. There is a test case for this scenario:
```_Float16 f16_val_7 = static_cast<_Float16>(1.0bf16); // expected-error
{{static_cast from '__bf16' to '_Float16' is not allowed}}```
If you still believe it might be better to add an assert I can do that, please
let me know.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:1235-1237
+ if (order == FRCR_Unordered) {
+ return QualType();
+ }
----------------
tahonermann wrote:
> Return of an invalid type is a new behavior for this function and it isn't
> clear to me that callers will handle it (or be expected to handle it) such
> that a diagnostic will be generated. Perhaps a diagnostic should be issued
> here? Or perhaps this should be an assertion?
It results in an error, please see the below test case:
```auto f16_bf16 = 1.0f16 + 1.0bf16; // expected-error {{invalid operands to
binary expression ('_Float16' and '__bf16')}}```
This function is only called by `UsualArithmeticConversions` which returns
invalid type in several places, hence returning invalid type should be ok? If
you still prefer an assertion or diagnostic, I will happily add one.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:4202
+ if (S.Context.getFloatingTypeOrder(SCS2.getToType(1),
+ SCS1.getFromType()) < FRCR_Equal)
{
+ return ImplicitConversionSequence::Better;
----------------
tahonermann wrote:
> This comparison includes `FRCR_Unordered`, is that intended? (another case at
> line 4225 below)
Yes, my understanding is we are checking if the second sequence's type's (T3)
conversion rank is not equal to the conversion rank of FP2. Is that not correct?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149573/new/
https://reviews.llvm.org/D149573
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
