On Fri, Sep 6, 2024 at 7:31 PM Jin Ma <ji...@linux.alibaba.com> wrote:
>
> When we use flto, the function list of rvv will be generated twice,
> once in the cc1 phase and once in the lto phase. However, due to
> the different generation methods, the two lists are different.
>
> For example, when there is no zvfh or zvfhmin in arch, it is
> generated by calling function "riscv_pragma_intrinsic". since the
> TARGET_VECTOR_ELEN_FP_16 is enabled before rvv function generation,
> a list of rvv functions related to float16 will be generated. In
> the lto phase, the rvv function list is generated only by calling
> the function "riscv_init_builtins", but the TARGET_VECTOR_ELEN_FP_16
> is disabled, so that the float16-related rvv function list cannot
> be generated like cc1. This will cause confusion, resulting in
> matching tothe wrong function due to inconsistent fcode in the lto
> phase, eventually leading to ICE.
>
> So I think we should be consistent with their generated lists, which
> is exactly what this patch does.
>
> But there is still a problem here. If we use "-fchecking", we still
> have ICE. This is because in the lto phase, after the rvv function
> list is generated and before the expand_builtin, the ggc_grow will
> be called to clean up the memory, resulting in
> "(* registered_functions)[code]->decl" being cleaned up to
> "<ggc_freed 0x7ffff6830c00>, and finally ICE".
>
> I think this is wrong and needs to be fixed, maybe we shouldn't
> use "ggc_alloc<registered_function> ()", or is there another better
> way to implement it?
>From the root we're marking the registered_functions vector via
the
template<typename T>
void
gt_ggc_mx (vec<T, va_gc> *v)
overload which will eventually mark registered_function * but since
you do not provide a gt_ggc_mx overload for the pointer type
this pointer will _not_ be marked.
> I'm trying to fix it here. Any comments here?
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-c.cc (struct pragma_intrinsic_flags): Mov
> to riscv-protos.h.
> (riscv_pragma_intrinsic_flags_pollute): Mov to
> riscv-vector-builtins.c.
> (riscv_pragma_intrinsic_flags_restore): Likewise.
> (riscv_pragma_intrinsic): Likewise.
> * config/riscv/riscv-protos.h (struct pragma_intrinsic_flags):
> New.
> (riscv_pragma_intrinsic_flags_restore): New.
> (riscv_pragma_intrinsic_flags_pollute): New.
> * config/riscv/riscv-vector-builtins.cc
> (riscv_pragma_intrinsic_flags_pollute): New.
> (riscv_pragma_intrinsic_flags_restore): New.
> (handle_pragma_vector_for_lto): New.
> (init_builtins): Correct the processing logic for lto.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/base/bug-10.c: New test.
> ---
> gcc/config/riscv/riscv-c.cc | 70 +---------------
> gcc/config/riscv/riscv-protos.h | 13 +++
> gcc/config/riscv/riscv-vector-builtins.cc | 83 ++++++++++++++++++-
> .../gcc.target/riscv/rvv/base/bug-10.c | 18 ++++
> 4 files changed, 114 insertions(+), 70 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/bug-10.c
>
> diff --git a/gcc/config/riscv/riscv-c.cc b/gcc/config/riscv/riscv-c.cc
> index 71112d9c66d7..7037ecc1268a 100644
> --- a/gcc/config/riscv/riscv-c.cc
> +++ b/gcc/config/riscv/riscv-c.cc
> @@ -34,72 +34,6 @@ along with GCC; see the file COPYING3. If not see
>
> #define builtin_define(TXT) cpp_define (pfile, TXT)
>
> -struct pragma_intrinsic_flags
> -{
> - int intrinsic_target_flags;
> -
> - int intrinsic_riscv_vector_elen_flags;
> - int intrinsic_riscv_zvl_flags;
> - int intrinsic_riscv_zvb_subext;
> - int intrinsic_riscv_zvk_subext;
> -};
> -
> -static void
> -riscv_pragma_intrinsic_flags_pollute (struct pragma_intrinsic_flags *flags)
> -{
> - flags->intrinsic_target_flags = target_flags;
> - flags->intrinsic_riscv_vector_elen_flags = riscv_vector_elen_flags;
> - flags->intrinsic_riscv_zvl_flags = riscv_zvl_flags;
> - flags->intrinsic_riscv_zvb_subext = riscv_zvb_subext;
> - flags->intrinsic_riscv_zvk_subext = riscv_zvk_subext;
> -
> - target_flags = target_flags
> - | MASK_VECTOR;
> -
> - riscv_zvl_flags = riscv_zvl_flags
> - | MASK_ZVL32B
> - | MASK_ZVL64B
> - | MASK_ZVL128B;
> -
> - riscv_vector_elen_flags = riscv_vector_elen_flags
> - | MASK_VECTOR_ELEN_32
> - | MASK_VECTOR_ELEN_64
> - | MASK_VECTOR_ELEN_FP_16
> - | MASK_VECTOR_ELEN_FP_32
> - | MASK_VECTOR_ELEN_FP_64;
> -
> - riscv_zvb_subext = riscv_zvb_subext
> - | MASK_ZVBB
> - | MASK_ZVBC
> - | MASK_ZVKB;
> -
> - riscv_zvk_subext = riscv_zvk_subext
> - | MASK_ZVKG
> - | MASK_ZVKNED
> - | MASK_ZVKNHA
> - | MASK_ZVKNHB
> - | MASK_ZVKSED
> - | MASK_ZVKSH
> - | MASK_ZVKN
> - | MASK_ZVKNC
> - | MASK_ZVKNG
> - | MASK_ZVKS
> - | MASK_ZVKSC
> - | MASK_ZVKSG
> - | MASK_ZVKT;
> -}
> -
> -static void
> -riscv_pragma_intrinsic_flags_restore (struct pragma_intrinsic_flags *flags)
> -{
> - target_flags = flags->intrinsic_target_flags;
> -
> - riscv_vector_elen_flags = flags->intrinsic_riscv_vector_elen_flags;
> - riscv_zvl_flags = flags->intrinsic_riscv_zvl_flags;
> - riscv_zvb_subext = flags->intrinsic_riscv_zvb_subext;
> - riscv_zvk_subext = flags->intrinsic_riscv_zvk_subext;
> -}
> -
> static int
> riscv_ext_version_value (unsigned major, unsigned minor)
> {
> @@ -269,14 +203,14 @@ riscv_pragma_intrinsic (cpp_reader *)
> {
> struct pragma_intrinsic_flags backup_flags;
>
> - riscv_pragma_intrinsic_flags_pollute (&backup_flags);
> + riscv_vector::riscv_pragma_intrinsic_flags_pollute (&backup_flags);
>
> riscv_option_override ();
> init_adjust_machine_modes ();
> riscv_vector::reinit_builtins ();
> riscv_vector::handle_pragma_vector ();
>
> - riscv_pragma_intrinsic_flags_restore (&backup_flags);
> + riscv_vector::riscv_pragma_intrinsic_flags_restore (&backup_flags);
>
> /* Re-initialize after the flags are restored. */
> riscv_option_override ();
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> index 3358e3887b95..651df2310da6 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -102,6 +102,15 @@ struct riscv_address_info {
> int shift;
> };
>
> +struct pragma_intrinsic_flags
> +{
> + int intrinsic_target_flags;
> + int intrinsic_riscv_vector_elen_flags;
> + int intrinsic_riscv_zvl_flags;
> + int intrinsic_riscv_zvb_subext;
> + int intrinsic_riscv_zvk_subext;
> +};
> +
> /* Routines implemented in riscv.cc. */
> extern const char *riscv_asm_output_opcode (FILE *asm_out_file, const char
> *p);
> extern enum riscv_symbol_type riscv_classify_symbolic_expression (rtx);
> @@ -569,6 +578,10 @@ enum avl_type
> VLS = 2,
> };
> /* Routines implemented in riscv-vector-builtins.cc. */
> +void
> +riscv_pragma_intrinsic_flags_restore (struct pragma_intrinsic_flags *);
> +void
> +riscv_pragma_intrinsic_flags_pollute (struct pragma_intrinsic_flags *);
> void init_builtins (void);
> void reinit_builtins (void);
> const char *mangle_builtin_type (const_tree);
> diff --git a/gcc/config/riscv/riscv-vector-builtins.cc
> b/gcc/config/riscv/riscv-vector-builtins.cc
> index 41730c483ee1..c6ddbeea71e7 100644
> --- a/gcc/config/riscv/riscv-vector-builtins.cc
> +++ b/gcc/config/riscv/riscv-vector-builtins.cc
> @@ -4505,6 +4505,83 @@ builtin_type_p (const_tree type)
> return lookup_vector_type_attribute (type);
> }
>
> +void
> +riscv_pragma_intrinsic_flags_pollute (struct pragma_intrinsic_flags *flags)
> +{
> + flags->intrinsic_target_flags = target_flags;
> + flags->intrinsic_riscv_vector_elen_flags = riscv_vector_elen_flags;
> + flags->intrinsic_riscv_zvl_flags = riscv_zvl_flags;
> + flags->intrinsic_riscv_zvb_subext = riscv_zvb_subext;
> + flags->intrinsic_riscv_zvk_subext = riscv_zvk_subext;
> +
> + target_flags = target_flags
> + | MASK_VECTOR;
> +
> + riscv_zvl_flags = riscv_zvl_flags
> + | MASK_ZVL32B
> + | MASK_ZVL64B
> + | MASK_ZVL128B;
> +
> + riscv_vector_elen_flags = riscv_vector_elen_flags
> + | MASK_VECTOR_ELEN_32
> + | MASK_VECTOR_ELEN_64
> + | MASK_VECTOR_ELEN_FP_16
> + | MASK_VECTOR_ELEN_FP_32
> + | MASK_VECTOR_ELEN_FP_64;
> +
> + riscv_zvb_subext = riscv_zvb_subext
> + | MASK_ZVBB
> + | MASK_ZVBC
> + | MASK_ZVKB;
> +
> + riscv_zvk_subext = riscv_zvk_subext
> + | MASK_ZVKG
> + | MASK_ZVKNED
> + | MASK_ZVKNHA
> + | MASK_ZVKNHB
> + | MASK_ZVKSED
> + | MASK_ZVKSH
> + | MASK_ZVKN
> + | MASK_ZVKNC
> + | MASK_ZVKNG
> + | MASK_ZVKS
> + | MASK_ZVKSC
> + | MASK_ZVKSG
> + | MASK_ZVKT;
> +}
> +
> +void
> +riscv_pragma_intrinsic_flags_restore (struct pragma_intrinsic_flags *flags)
> +{
> + target_flags = flags->intrinsic_target_flags;
> +
> + riscv_vector_elen_flags = flags->intrinsic_riscv_vector_elen_flags;
> + riscv_zvl_flags = flags->intrinsic_riscv_zvl_flags;
> + riscv_zvb_subext = flags->intrinsic_riscv_zvb_subext;
> + riscv_zvk_subext = flags->intrinsic_riscv_zvk_subext;
> +}
> +
> +/* Helper for init_builtins in LTO. */
> +static void
> +handle_pragma_vector_for_lto ()
> +{
> + struct pragma_intrinsic_flags backup_flags;
> +
> + riscv_pragma_intrinsic_flags_pollute (&backup_flags);
> +
> + riscv_option_override ();
> + init_adjust_machine_modes ();
> +
> + register_builtin_types ();
> +
> + handle_pragma_vector ();
> + riscv_pragma_intrinsic_flags_restore (&backup_flags);
> +
> + /* Re-initialize after the flags are restored. */
> + riscv_option_override ();
> + init_adjust_machine_modes ();
> +}
> +
> /* Initialize all compiler built-ins related to RVV that should be
> defined at start-up. */
> void
> @@ -4513,9 +4590,11 @@ init_builtins ()
> rvv_switcher rvv;
> if (!TARGET_VECTOR)
> return;
> - register_builtin_types ();
> +
> if (in_lto_p)
> - handle_pragma_vector ();
> + handle_pragma_vector_for_lto ();
> + else
> + register_builtin_types ();
> }
>
> /* Reinitialize builtins similar to init_builtins, but only the null
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/bug-10.c
> b/gcc/testsuite/gcc.target/riscv/rvv/base/bug-10.c
> new file mode 100644
> index 000000000000..c6b49da0768e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/bug-10.c
> @@ -0,0 +1,18 @@
> +/* Test that we do not have ice when compile */
> +
> +/* { dg-do run } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -mrvv-vector-bits=zvl -flto -O2
> -fno-checking" } */
> +
> +#include <riscv_vector.h>
> +
> +int
> +main ()
> +{
> + size_t vl = 8;
> + vint32m1_t vs1 = {};
> + vint32m1_t vs2 = {};
> +
> + __volatile__ vint32m1_t vd = __riscv_vadd_vv_i32m1(vs1, vs2, vl);
> +
> + return 0;
> +}
> --
> 2.17.1
>
