stuij marked 4 inline comments as done.
stuij added a comment.
@LukeGeeson: you're already mentioned :) See the commits tab on this review.
But Simon Tatham needs a mention as well. I shall add him.
================
Comment at: clang/docs/LanguageExtensions.rst:518
+Clang supports three half-precision (16-bit) floating point types: ``__fp16``,
+``_Float16`` and ``__bf16``. These types are supported in all language modes.
----------------
SjoerdMeijer wrote:
> Not my field of expertise, and sorry if I've missed this somewhere, but was
> wondering if this (i.e. all language modes) means we need C++ name mangling
> support for bfloat? In the AArch64 backend I saw this:
>
> getBFloat16Mangling() const override { return "u6__bf16"; }
>
> But that's something else I guess, and I was guessing a 2 letter mangling
> needs to be added here?
>
> https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling-builtin
Yes, we need name-mangling support, and yes that method you mentioned does
that. There's one for AArch32 as well. And yes we have documented it in the
'C++ ABI for the latest Arm Architecture' docs:
aarch32: https://developer.arm.com/docs/ihi0041/latest
aarch64: https://developer.arm.com/docs/ihi0059/latest
================
Comment at: clang/include/clang-c/Index.h:3254
CXType_FirstBuiltin = CXType_Void,
CXType_LastBuiltin = CXType_ULongAccum,
----------------
majnemer wrote:
> Should this be:
> CXType_LastBuiltin = CXType_BFloat16,
>
thanks
================
Comment at: clang/lib/AST/ItaniumMangle.cpp:3186
+ case BuiltinType::Half: EltName = "float16_t"; break;
+ case BuiltinType::BFloat16: EltName = "bfloat16x1_t"; break;
default:
----------------
majnemer wrote:
> Why is this x1?
Yes, that does look odd. The original author of this code has left the company,
but I'll ask around.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:1873-1874
// We of course allow this conversion if long double is really double.
+ if (FromType == S.Context.BFloat16Ty || ToType == S.Context.BFloat16Ty)
+ return false;
if (&S.Context.getFloatTypeSemantics(FromType) !=
----------------
majnemer wrote:
> This probably needs an explanation.
done
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76077/new/
https://reviews.llvm.org/D76077
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
