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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
