-
Notifications
You must be signed in to change notification settings - Fork 163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add ABI information for __bf16 #367
Conversation
I have a strong preference to Regarding |
Unlike But So the PR generally LGTM, but I would defer the formal LGTM until we've PoC implementation posted :) NOTE: actually I was preferring |
…like x86_64 and arm. According to https://github.com/riscv/riscv-bfloat16 , zfbfmin extension depends on zfh/zfhmin extension. According to the discussion riscv-non-isa/riscv-elf-psabi-doc#367, this use __bf16 and use DF16b in riscv_mangle_type like x86. gcc\ChangeLog: * common/config/riscv/riscv-common.cc: Add ZFBFMIN extension. * config/riscv/iterators.md (TARGET_ZFHMIN):Add iterator BF. (fld):Likewise. (ld):Likewise (fsd):Likewise (sd):Likewise (d):Likewise (DF):Likewise * config/riscv/riscv-builtins.cc (riscv_init_builtin_types): Add bfloat16 type in riscv . (riscv_fp16_builtin_type):Likewise (riscv_bf16_builtin_type):Likewise * config/riscv/riscv-modes.def (FLOAT_MODE):Likewise (ADJUST_FLOAT_FORMAT):Likewise * config/riscv/riscv-opts.h (MASK_ZFBFMIN):Add ZFBFMIN extension. (TARGET_ZFBFMIN): * config/riscv/riscv-vector-switch.def (ENTRY): * config/riscv/riscv.cc (riscv_emit_float_compare):Add bfloat16 type in riscv . (riscv_mangle_type): (riscv_scalar_mode_supported_p): (riscv_libgcc_floating_mode_supported_p): (riscv_init_libfuncs): * config/riscv/riscv.md (mode" ):Add bfloat16 type in riscv . (truncdfhf2): (truncsfbf2): (truncdf<BFHF:mode>2): (extendbfsf2): (extendhfdf2): (extend<BFHF:mode>df2): (movbf): (*movbf_hardfloat): (*movbf_softfloat): libgcc\ChangeLog: * config/riscv/sfp-machine.h (_FP_NANFRAC_B):Add bfloat16 type in riscv . (_FP_NANSIGN_B): * config/riscv/t-softfp32:
I have a question. I had sent a gcc patch to support bfloat16 in riscv in https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613499.html |
@Liaoshihua good catch, thanks for raising that issue! That's the list for related discussion I collected so far (not complete list but I put most important 3 here): |
While BFloat16 (BF16) is not a standard format specified in IEEE-754, it is a floating-point format as defined by the standard. As such, it is characterized by its radix (2), precision (8-bit mantissa) and its exponent rage (emax=127). It follows all of the rules for IEEE conformance. In RISC-V (and many other architectures), BF16 is both an interchange and an arithmetic format. |
Thank you for pointing out the various discussions about mangling. Currently the RISC-V psABI doesn't specify its own mangling rules and instead points to the Itanium C++ ABI. I think it would make sense to maintain this property, so perhaps his PR is blocked until itanium-cxx-abi/cxx-abi#147 is resolved? |
@asb Yes, I would prefer wait itanium-cxx-abi/cxx-abi#147 get resolved, at least for the |
I'd like to question one thing that's come up in these discussions - both your psABI meeting slides and Ken's response indicate that bf16 is both an interchange and an arithmetic format on RISC-V. Is that really the case though? Zfbfmin only defines load/store and conversion instructions. |
Yes, it is an arithmetic format too; the proposed Zvfbfwma extension defines a widening floating-point fused multiply-add. Draft BF16 20230322 for AR was released to the Architectural Review committee as the first step towards freeze. |
Yes, but that is of course a very limited set of operations compared to 32-bit floats, and a system might only implement Zfbfmin. Different groups are perhaps using differing terminology here, but on some of the linked issues the "storage format" vs "arithmetic format" distinction appears is used with respect to whether you can perform arithmetic operations on |
I would argue that Zvfbfwma does operate on BF16 in that it consumes values in this format. |
After reading more material, I prefer to define bf16 as an interchange and an arithmetic format, even NO HW instructions are provided. Let me use x86's ABI bf16 motivation[1] here:
For the same reason as x86, we should define _bf16 and it could also be used as C++23 std::bfloat16 type. So here are questions about arithmetic, should we provide _bf16 operation libcall? Let's break that into several parts:
One concern might be that the clang/LLVM is forbidden any operations on __bf16. Still, I believe it will relax soon since C++23 allows operation on std::bfloat16 unless clang/LLVM defines a different underlying type for std::bloat16. And also GCC already allow the operation for __bf16, and provide libgcc function for bfloat16 type. So, in summary - defining bf16 as an interchange and an arithmetic format is the right way to go, I believe - it's the foundation of implementing C++ std::bfloat16 type, and referring to AArch64 and x86 experience, we should go to define that in that way. Otherwise, we might change that later. Refs: |
I think you mean FP32, but the comment still needs clarification regarding behavior with NaNs. There are competing definitions of BF16, and BF16 isn't (yet) explicitly defined in IEEE-754. But IEEE-754 does provide a framework for defining BF16 as a new binary arithmetic and interchange type. And in that framework, a BF16 signalling NaN must raise NV on conversion to FP32 and must return a quiet NaN. Additionally, following RISC-V (implementation-defined) behavior for other floating-point conversions, if the input to a BF16 to FP32 conversion is any kind of NaN, the output should be |
It seems that the std::bfloat16_t part in itanium-cxx-abi/cxx-abi#147 has been merged. Can we now continue addressing this PR and finalize it? @kito-cheng @asb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As Nick pointed out, I do have some concerns regarding the bfloat16 arithmetic. However, this PR primarily describes the alignment and size, which I believe can be separated from the computational details.
So LGTM.
Oh, wait, @asb could you add one more sentence in C++ Name Mangling to mention |
Yes I saw that linked PR finally got resolved - thanks for the nudge. I'll re-review and update this patch tomorrow. |
I think this is now ready for merging - I've added a note that
|
Thanks! |
The RISC-V psABI recently added __bf16 in riscv-non-isa/riscv-elf-psabi-doc#367. Now we can enable this new type in clang. Reviewed By: craig.topper Differential Revision: https://reviews.llvm.org/D150929
The RISC-V psABI recently added __bf16 in riscv-non-isa/riscv-elf-psabi-doc#367. Now we can enable this new type in clang. Reviewed By: craig.topper Differential Revision: https://reviews.llvm.org/D150929
The RISC-V psABI recently added __bf16 in riscv-non-isa/riscv-elf-psabi-doc#367. Now we can enable this new type in clang. Reviewed By: craig.topper Differential Revision: https://reviews.llvm.org/D150929
1 At point <https://github.com/riscv/riscv-bfloat16>, BF16 has already been completed "post public review". 2 LLVM has also added support for RISCV BF16 in <https://reviews.llvm.org/D151313> and <https://reviews.llvm.org/D150929>. 3 According to the discussion <riscv-non-isa/riscv-elf-psabi-doc#367>, this use __bf16 and use DF16b in riscv_mangle_type like x86. Below test are passed for this patch * The riscv fully regression test. gcc/ChangeLog: * config/riscv/iterators.md: New mode iterator HFBF. * config/riscv/riscv-builtins.cc (riscv_init_builtin_types): Initialize data type _Bfloat16. * config/riscv/riscv-modes.def (FLOAT_MODE): New. (ADJUST_FLOAT_FORMAT): New. * config/riscv/riscv.cc (riscv_mangle_type): Support for BFmode. (riscv_scalar_mode_supported_p): Ditto. (riscv_libgcc_floating_mode_supported_p): Ditto. (riscv_init_libfuncs): Set the conversion method for BFmode and HFmode. (riscv_block_arith_comp_libfuncs_for_mode): Set the arithmetic and comparison libfuncs for the mode. * config/riscv/riscv.md (mode" ): Add BF. (movhf): Support for BFmode. (mov<mode>): Ditto. (*movhf_softfloat): Ditto. (*mov<mode>_softfloat): Ditto. libgcc/ChangeLog: * config/riscv/sfp-machine.h (_FP_NANFRAC_B): New. (_FP_NANSIGN_B): Ditto. * config/riscv/t-softfp32: Add support for BF16 libfuncs. * config/riscv/t-softfp64: Ditto. * soft-fp/floatsibf.c: For si -> bf16. * soft-fp/floatunsibf.c: For unsi -> bf16. gcc/testsuite/ChangeLog: * gcc.target/riscv/bf16_arithmetic.c: New test. * gcc.target/riscv/bf16_call.c: New test. * gcc.target/riscv/bf16_comparison.c: New test. * gcc.target/riscv/bf16_float_libcall_convert.c: New test. * gcc.target/riscv/bf16_integer_libcall_convert.c: New test. Co-authored-by: Jin Ma <jinma@linux.alibaba.com>
1 At point <https://github.com/riscv/riscv-bfloat16>, BF16 has already been completed "post public review". 2 LLVM has also added support for RISCV BF16 in <https://reviews.llvm.org/D151313> and <https://reviews.llvm.org/D150929>. 3 According to the discussion <riscv-non-isa/riscv-elf-psabi-doc#367>, this use __bf16 and use DF16b in riscv_mangle_type like x86. Below test are passed for this patch * The riscv fully regression test. gcc/ChangeLog: * config/riscv/iterators.md: New mode iterator HFBF. * config/riscv/riscv-builtins.cc (riscv_init_builtin_types): Initialize data type _Bfloat16. * config/riscv/riscv-modes.def (FLOAT_MODE): New. (ADJUST_FLOAT_FORMAT): New. * config/riscv/riscv.cc (riscv_mangle_type): Support for BFmode. (riscv_scalar_mode_supported_p): Ditto. (riscv_libgcc_floating_mode_supported_p): Ditto. (riscv_init_libfuncs): Set the conversion method for BFmode and HFmode. (riscv_block_arith_comp_libfuncs_for_mode): Set the arithmetic and comparison libfuncs for the mode. * config/riscv/riscv.md (mode" ): Add BF. (movhf): Support for BFmode. (mov<mode>): Ditto. (*movhf_softfloat): Ditto. (*mov<mode>_softfloat): Ditto. libgcc/ChangeLog: * config/riscv/sfp-machine.h (_FP_NANFRAC_B): New. (_FP_NANSIGN_B): Ditto. * config/riscv/t-softfp32: Add support for BF16 libfuncs. * config/riscv/t-softfp64: Ditto. * soft-fp/floatsibf.c: For si -> bf16. * soft-fp/floatunsibf.c: For unsi -> bf16. gcc/testsuite/ChangeLog: * gcc.target/riscv/bf16_arithmetic.c: New test. * gcc.target/riscv/bf16_call.c: New test. * gcc.target/riscv/bf16_comparison.c: New test. * gcc.target/riscv/bf16_float_libcall_convert.c: New test. * gcc.target/riscv/bf16_integer_libcall_convert.c: New test. Co-authored-by: Jin Ma <jinma@linux.alibaba.com> (cherry picked from commit 8c7cee8)
According to the description in: <riscv-non-isa/riscv-elf-psabi-doc#367>, the type representation symbol of BF16 has been corrected. Kito Cheng pointed out relevant information in the email: <https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651850.html> gcc/ChangeLog: * config/riscv/riscv-builtins.cc (riscv_init_builtin_types): Modify _Bfloat16 to __bf16. * config/riscv/riscv.cc (riscv_mangle_type): Ditto. gcc/testsuite/ChangeLog: * gcc.target/riscv/_Bfloat16-nanboxing.c: Move to... * gcc.target/riscv/__bf16-nanboxing.c: ...here. * gcc.target/riscv/bf16_arithmetic.c: Modify _Bfloat16 to __bf16. * gcc.target/riscv/bf16_call.c: Ditto. * gcc.target/riscv/bf16_comparison.c: Ditto. * gcc.target/riscv/bf16_float_libcall_convert.c: Ditto. * gcc.target/riscv/bf16_integer_libcall_convert.c: Ditto.
According to the description in: <riscv-non-isa/riscv-elf-psabi-doc#367>, the type representation symbol of BF16 has been corrected. Kito Cheng pointed out relevant information in the email: <https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651850.html> gcc/ChangeLog: * config/riscv/riscv-builtins.cc (riscv_init_builtin_types): Modify _Bfloat16 to __bf16. * config/riscv/riscv.cc (riscv_mangle_type): Ditto. gcc/testsuite/ChangeLog: * gcc.target/riscv/_Bfloat16-nanboxing.c: Move to... * gcc.target/riscv/__bf16-nanboxing.c: ...here. * gcc.target/riscv/bf16_arithmetic.c: Modify _Bfloat16 to __bf16. * gcc.target/riscv/bf16_call.c: Ditto. * gcc.target/riscv/bf16_comparison.c: Ditto. * gcc.target/riscv/bf16_float_libcall_convert.c: Ditto. * gcc.target/riscv/bf16_integer_libcall_convert.c: Ditto. (cherry picked from commit 6da1d6e)
1 At point <https://github.com/riscv/riscv-bfloat16>, BF16 has already been completed "post public review". 2 LLVM has also added support for RISCV BF16 in <https://reviews.llvm.org/D151313> and <https://reviews.llvm.org/D150929>. 3 According to the discussion <riscv-non-isa/riscv-elf-psabi-doc#367>, this use __bf16 and use DF16b in riscv_mangle_type like x86. Below test are passed for this patch * The riscv fully regression test. gcc/ChangeLog: * config/riscv/iterators.md: New mode iterator HFBF. * config/riscv/riscv-builtins.cc (riscv_init_builtin_types): Initialize data type _Bfloat16. * config/riscv/riscv-modes.def (FLOAT_MODE): New. (ADJUST_FLOAT_FORMAT): New. * config/riscv/riscv.cc (riscv_mangle_type): Support for BFmode. (riscv_scalar_mode_supported_p): Ditto. (riscv_libgcc_floating_mode_supported_p): Ditto. (riscv_init_libfuncs): Set the conversion method for BFmode and HFmode. (riscv_block_arith_comp_libfuncs_for_mode): Set the arithmetic and comparison libfuncs for the mode. * config/riscv/riscv.md (mode" ): Add BF. (movhf): Support for BFmode. (mov<mode>): Ditto. (*movhf_softfloat): Ditto. (*mov<mode>_softfloat): Ditto. libgcc/ChangeLog: * config/riscv/sfp-machine.h (_FP_NANFRAC_B): New. (_FP_NANSIGN_B): Ditto. * config/riscv/t-softfp32: Add support for BF16 libfuncs. * config/riscv/t-softfp64: Ditto. * soft-fp/floatsibf.c: For si -> bf16. * soft-fp/floatunsibf.c: For unsi -> bf16. gcc/testsuite/ChangeLog: * gcc.target/riscv/bf16_arithmetic.c: New test. * gcc.target/riscv/bf16_call.c: New test. * gcc.target/riscv/bf16_comparison.c: New test. * gcc.target/riscv/bf16_float_libcall_convert.c: New test. * gcc.target/riscv/bf16_integer_libcall_convert.c: New test. Co-authored-by: Jin Ma <jinma@linux.alibaba.com> (cherry picked from commit 8c7cee8)
According to the description in: <riscv-non-isa/riscv-elf-psabi-doc#367>, the type representation symbol of BF16 has been corrected. Kito Cheng pointed out relevant information in the email: <https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651850.html> gcc/ChangeLog: * config/riscv/riscv-builtins.cc (riscv_init_builtin_types): Modify _Bfloat16 to __bf16. * config/riscv/riscv.cc (riscv_mangle_type): Ditto. gcc/testsuite/ChangeLog: * gcc.target/riscv/_Bfloat16-nanboxing.c: Move to... * gcc.target/riscv/__bf16-nanboxing.c: ...here. * gcc.target/riscv/bf16_arithmetic.c: Modify _Bfloat16 to __bf16. * gcc.target/riscv/bf16_call.c: Ditto. * gcc.target/riscv/bf16_comparison.c: Ditto. * gcc.target/riscv/bf16_float_libcall_convert.c: Ditto. * gcc.target/riscv/bf16_integer_libcall_convert.c: Ditto. (cherry picked from commit 6da1d6e)
At this point, posted just for comment. We'll need something covering this with the upcoming RISC-V bfloat16 extension. I think this is all that is needed. For reference, see the equivalent change in the x86-64 psABI.
I haven't carefully followed
__fp16
vs_Float16
and_BFloat16
vs__bf16
. It looks like__bf16
is preferred on other targets. Though this would also be a good moment to discuss that further if people feel that's not the right choice for RISC-V.My understanding is that
_Float16
and__bf16
will be passed in FPRs in exactly the same cases an FPR would be used for afloat
parameter or struct field. Again - please speak up if you feel that isn't what the psABI states.