[PATCH] D150913: [Clang][Bfloat16] Upgrade __bf16 to arithmetic type, change mangling, and extend excess precision support.

[M. Zeeshan Siddiqui via Phabricator via cfe-commits]

codemzs marked an inline comment as done.
codemzs added a comment.

I believe I had updated the `__bf16` documentation in `/llvm-project/clang/docs/LanguageExtensions.rst`, but it appears to have been omitted in this patch. I assure you, I'll rectify this in the next iteration.



================
Comment at: clang/include/clang/AST/ASTContext.h:1102
   CanQualType HalfTy; // [OpenCL 6.1.1.1], ARM NEON
-  CanQualType BFloat16Ty;
+  CanQualType BFloat16Ty; // ISO/IEC/IEEE 60559.
   CanQualType Float16Ty; // C11 extension ISO/IEC TS 18661-3
----------------
pengfei wrote:
> Don't have a look at ISO/IEC/IEEE 60559, but I doubt BF16 is still not a IEEE type for now.
You are correct, it isn't officially part of ISO/IEEEE standard but implements the properties specified by the standard I think, in any case I will remove the comment as it could be misleading.


================
Comment at: clang/lib/Basic/Targets/AMDGPU.h:121
   bool hasBFloat16Type() const override { return isAMDGCN(getTriple()); }
-  const char *getBFloat16Mangling() const override { return "u6__bf16"; };
+  const char *getBFloat16Mangling() const override { return "DF16b"; };
 
----------------
pengfei wrote:
> I think it's time to bring D139608 back with this patch :)
I'm inclined to establish a default value, overridden only for ARM, to avoid repetition. If there are no objections, I plan to implement this change in the next iteration.


================
Comment at: clang/lib/Basic/Targets/X86.cpp:381
 
     HasBFloat16 = SSELevel >= SSE2;
 
----------------
rjmccall wrote:
> pengfei wrote:
> > I'm not sure if I understand the meaning of `HasFullBFloat16`. If it is used for target that supports arithmetic `__bf16`, we should not use `+fullbf16` but always enable it for SSE2, i.e., `HasFullBFloat16 = SSELevel >= SSE2`. Because X86 GCC already supports arithmetic for `__bf16`.
> > 
> > If this is used in the way like `HasLegalHalfType`, we should enable it once we have a full BF16 ISA on X86. `fullbf16` doesn't make much sense to me.
> At the moment, we haven't done the work to emulate BFloat16 arithmetic in any of the three ways we can do that: Clang doesn't promote it in IRGen, LLVM doesn't promote it in legalization, and we don't have compiler-rt functions for it.  If we emit these instructions, they'll just sail through LLVM and fail in the backend.  So in the short term, we have to restrict this to targets that directly support BFloat16 arithmetic in hardware, which doesn't include x86.
> 
> Once we have that emulation support, I agree that the x86 targets should enable this whenever they would enable `__bf16`.
@rjmccall, I concur and just wanted to confirm this change indeed intends to provide `BFloat16` emulation support, utilizing excess precision for promotion to `float`. The `HasFullBFloat16` switch is designed to determine excess precision support automatically when the hardware does not natively support `bfloat16` arithmetic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150913/new/

https://reviews.llvm.org/D150913