zahiraam added inline comments.
================
Comment at: clang/lib/Basic/Targets/X86.cpp:381
HasBFloat16 = SSELevel >= SSE2;
----------------
codemzs wrote:
> 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.
> 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`.
Would be nice to add a comment to clarify it.
================
Comment at: clang/test/CodeGen/X86/bfloat16.cpp:2-3
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
UTC_ARGS: --version 2
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -target-feature +fullbf16
-S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - |
FileCheck -check-prefix=CHECK-NBF16 %s
+
----------------
pengfei wrote:
> The backend has already support lowering of `bfloat`, I don't think it's
> necessary to do extra work in FE unless for excess-precision.
> The backend has already support lowering of `bfloat`, I don't think it's
> necessary to do extra work in FE unless for excess-precision.
+1.
================
Comment at: clang/test/CodeGen/X86/fexcess-precision-bfloat16.c:361
+}
\ No newline at end of file
----------------
Fix this.
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
