On 5/10/2026 11:49 PM, Lino Hsing-Yu Peng wrote:
> Implement Zvfofp8min widening conversions from FP8 to BF16 across the RVV
> builtin framework and machine descriptions. This adds the float8 pattern
> definitions and wires new shapes, types, and unspecs for these operations.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-vector-builtins-bases.cc: Add f8 widening
> conversions.
> * config/riscv/riscv-vector-builtins-functions.def: Add zvfofp8min
> builtins.
> * config/riscv/riscv-vector-builtins-shapes.cc: Add f8 shapes and
> naming.
> * config/riscv/riscv-vector-builtins-shapes.h: Declare f8 shapes.
> * config/riscv/riscv-vector-builtins.cc: Add f8_to_bf16 operand info.
> * config/riscv/riscv-vector-builtins-types.def: Add bf16 ops list.
> * config/riscv/riscv.md: Add zvfofp8min unspecs and include
> vector-float8.md.
> * config/riscv/vector-float8.md: New file.
>
> Signed-off-by: Lino Hsing-Yu Peng <[email protected]>
> ---
> .../riscv/riscv-vector-builtins-bases.cc | 30 ++++++-
> .../riscv/riscv-vector-builtins-functions.def | 6 ++
> .../riscv/riscv-vector-builtins-shapes.cc | 88 +++++++++++++++++--
> .../riscv/riscv-vector-builtins-shapes.h | 3 +
> .../riscv/riscv-vector-builtins-types.def | 14 +++
> gcc/config/riscv/riscv-vector-builtins.cc | 14 +++
> gcc/config/riscv/riscv.md | 6 ++
> gcc/config/riscv/vector-float8.md | 63 +++++++++++++
> 8 files changed, 216 insertions(+), 8 deletions(-)
> create mode 100644 gcc/config/riscv/vector-float8.md
>
> diff --git a/gcc/config/riscv/riscv-vector-builtins-bases.cc
> b/gcc/config/riscv/riscv-vector-builtins-bases.cc
> index 525a622882a..58ab57db5d4 100644
> --- a/gcc/config/riscv/riscv-vector-builtins-bases.cc
> +++ b/gcc/config/riscv/riscv-vector-builtins-bases.cc
> @@ -1516,13 +1516,41 @@ public:
> }
> };
>
> +enum altfmt
> +{
> + F8NONE,
> + F8E4M3,
> + F8E5M2
> +};
> +
> +static altfmt
> +get_altfmt (const function_expander &e)
So in general, every function/method should have a function comment
indicating what it does. They typically say something about the
arguments and return value. So...
/* Return the ALTFMT associated with E's shape. If there is no matching
ALTFMT, then
return F8NONE. */
Or something along those lines. The idea is someone ought to be able to
look at that comment and determine what the function does and how to use
it without diving into the implementation details. This is a pretty
simple function, but as things get more complex those function comments
really help folks navigate the sources.
> +{
> + if (e.shape == shapes::alu_f8e4m3)
> + return F8E4M3;
> + if (e.shape == shapes::alu_f8e5m2)
> + return F8E5M2;
> + return F8NONE;
> +}
> class vfwcvt_f : public function_base
You probably want a newline between the close brace and class definition.
> diff --git a/gcc/config/riscv/riscv-vector-builtins-shapes.cc
> b/gcc/config/riscv/riscv-vector-builtins-shapes.cc
> index 3bf40c432c2..3533ef01714 100644
> --- a/gcc/config/riscv/riscv-vector-builtins-shapes.cc
> +++ b/gcc/config/riscv/riscv-vector-builtins-shapes.cc
> @@ -89,15 +89,14 @@ supports_vectype_p (const function_group_info &group,
> unsigned int vec_type_idx)
> /* Only judge for bf16 vector type */
Given this would deal with f8 after your change, the comment above needs
updating.
> if (*group.shape == shapes::loadstore
> || *group.shape == shapes::indexed_loadstore
> - || *group.shape == shapes::vundefined
> - || *group.shape == shapes::misc
> - || *group.shape == shapes::vset
> - || *group.shape == shapes::vget
> - || *group.shape == shapes::vcreate
> - || *group.shape == shapes::fault_load
> + || *group.shape == shapes::vundefined || *group.shape == shapes::misc
> + || *group.shape == shapes::vset || *group.shape == shapes::vget
> + || *group.shape == shapes::vcreate || *group.shape ==
> shapes::fault_load
These looks like gratutious changes. Was there a reason to make the
changes above? If not, please put them back the way they were. If I
understand what you're trying to do in this code I think you just want
to add the f8e4m3/f8e5m2 cases and nothing else. Right?
> @@ -424,6 +423,78 @@ struct alu_def : public build_base
> }
> };
>
> +static void
> +append_f8_suffix (function_builder &b, vector_type_index vti,
> + const char *altfmt)
[ ... ]
Why is F8 special here? I don't see the same kind of special handling
for other cases. Robin and Juzhe know this stuff better than I do, but
it would be nice if someone not familiar with all this code could easily
understand what's going on and why F8 is special.
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 3fe0ad0ccdf..a5e5ecae3e7 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -103,6 +103,11 @@
> ;; Stack Smash Protector
> UNSPEC_SSP_SET
> UNSPEC_SSP_TEST
> + ;; Zvfofp8min
> + UNSPEC_F8E4M3
> + UNSPEC_F8E5M2
> + UNSPEC_F8E4M3_SAT
> + UNSPEC_F8E5M2_SAT
> ])
Are the _SAT variants used in a subsequent patch? If not it would seem
like they're extraneous.
>
> (define_c_enum "unspecv" [
>
> diff --git a/gcc/config/riscv/vector-float8.md
> b/gcc/config/riscv/vector-float8.md
> new file mode 100644
> index 00000000000..2a3eeeaa1db
> --- /dev/null
> +++ b/gcc/config/riscv/vector-float8.md
> @@ -0,0 +1,63 @@
> +;; Machine description for RISC-V Zvfofp8 extension conversions.
> +;; Copyright (C) 2026 Free Software Foundation, Inc.
> +
> +;; This file is part of GCC.
> +
> +;; GCC is free software; you can redistribute it and/or modify
> +;; it under the terms of the GNU General Public License as published by
> +;; the Free Software Foundation; either version 3, or (at your option)
> +;; any later version.
> +
> +;; GCC is distributed in the hope that it will be useful,
> +;; but WITHOUT ANY WARRANTY; without even the implied warranty of
> +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> +;; General Public License for more details.
> +
> +;; You should have received a copy of the GNU General Public License
> +;; along with GCC; see the file COPYING3. If not see
> +;; <http://www.gnu.org/licenses/>.
> +
> +(define_mode_iterator VWEXTF_ZVFOFP8MIN [
> + (RVVM8BF "TARGET_ZVFOFP8MIN && TARGET_ZVFBFMIN")
> + (RVVM4BF "TARGET_ZVFOFP8MIN && TARGET_ZVFBFMIN")
> + (RVVM2BF "TARGET_ZVFOFP8MIN && TARGET_ZVFBFMIN")
> + (RVVM1BF "TARGET_ZVFOFP8MIN && TARGET_ZVFBFMIN")
> + (RVVMF2BF "TARGET_ZVFOFP8MIN && TARGET_ZVFBFMIN")
> + (RVVMF4BF "TARGET_ZVFOFP8MIN && TARGET_ZVFBFMIN && TARGET_MIN_VLEN > 32")
> +])
> +
> +(define_mode_attr VBF_DOUBLE_TRUNC [
> + (RVVM8BF "RVVM4QI")
> + (RVVM4BF "RVVM2QI")
> + (RVVM2BF "RVVM1QI")
> + (RVVM1BF "RVVMF2QI")
> + (RVVMF2BF "RVVMF4QI")
> + (RVVMF4BF "RVVMF8QI")
> +])
> +
> +(define_int_iterator ALTFMT [UNSPEC_F8E4M3 UNSPEC_F8E5M2])
> +(define_int_attr altfmt
> + [(UNSPEC_F8E4M3 "f8e4m3")
> + (UNSPEC_F8E5M2 "f8e5m2")])
> +
> +;; Zvfofp8min extension: FP8 to BF16 widening conversions.
> +
> +(define_insn "@pred_extend_<altfmt>_to_<mode>"
> + [(set (match_operand:VWEXTF_ZVFOFP8MIN 0 "register_operand"
> "=&vr, &vr")
> + (if_then_else:VWEXTF_ZVFOFP8MIN
> + (unspec:<VM>
> + [(match_operand:<VM> 1 "vector_mask_operand" "vmWc1,vmWc1")
> + (match_operand 4 "vector_length_operand" " rK, rK")
> + (match_operand 5 "const_int_operand" " i, i")
> + (match_operand 6 "const_int_operand" " i, i")
> + (match_operand 7 "const_int_operand" " i, i")
> + (reg:SI VL_REGNUM)
> + (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE)
> + (unspec:VWEXTF_ZVFOFP8MIN
> + [(float_extend:VWEXTF_ZVFOFP8MIN
> + (match_operand:<VBF_DOUBLE_TRUNC> 3 "register_operand" " vr,
> vr"))] ALTFMT)
> + (match_operand:VWEXTF_ZVFOFP8MIN 2 "vector_merge_operand" "
> vu, 0")))]
> + "TARGET_VECTOR && TARGET_ZVFOFP8MIN && TARGET_ZVFBFMIN"
> + "vfwcvtbf16.f.f.v\t%0,%3%p1"
> + [(set_attr "type" "vfwcvtbf16")
> + (set_attr "mode" "<VBF_DOUBLE_TRUNC>")])
So we don't seem to adjust anything in the final assembly based on
ALTFMT?!? Is that right? If so, then what's the purpose of having
ALTFMT in the pattern to begin with?
Anyway, that's what I've noticed. Robin and Juzhe know this code better
than I and their comments/concerns should have priority over any of mine.
jeff
