diff mbox series

RISC-V: Fix ICE for rvv in lto

Message ID 20240906173047.306-1-jinma@linux.alibaba.com
State New
Headers show
Series RISC-V: Fix ICE for rvv in lto | expand

Commit Message

Jin Ma Sept. 6, 2024, 5:30 p.m. UTC
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?

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

Comments

Li, Pan2 Sept. 7, 2024, 4:40 a.m. UTC | #1
> +/* 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;
> +}

Interesting, do we still have ice when there is no __voltaile__ for vd? As well as gcc-14 branch.
Because it is quite a common case that should be covered by test already.

Pan

-----Original Message-----
From: Jin Ma <jinma@linux.alibaba.com> 
Sent: Saturday, September 7, 2024 1:31 AM
To: gcc-patches@gcc.gnu.org
Cc: jeffreyalaw@gmail.com; juzhe.zhong@rivai.ai; Li, Pan2 <pan2.li@intel.com>; kito.cheng@gmail.com; jinma.contrib@gmail.com; Jin Ma <jinma@linux.alibaba.com>
Subject: [PATCH] RISC-V: Fix ICE for rvv in lto

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?

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;
+}
Jin Ma Sept. 7, 2024, 9:42 a.m. UTC | #2
> > +/* 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;
> > +}
> 
> Interesting, do we still have ice when there is no __voltaile__ for vd? As well as gcc-14 branch.
> Because it is quite a common case that should be covered by test already.
> 
> Pan

Yes, I am also surprised that this kind of ICE will appear. It really should be covered by
test cases. But in fact, if we do not use zvfh or zvfhmin in arch, rvv cannot be used in LTO.

This has nothing to do with "__voltaile__". "__voltaile__" in the case is just that I want it
to be compiled to the end and not optimized.

In fact, a simple case can reproduce ICE, including gcc-14 and master, for example:

#include <riscv_vector.h>

vint32m1_t foo(vint32m1_t vs1, vint32m1_t vs2, size_t vl) 
{
  return __riscv_vadd_vv_i32m1(vs1, vs2, vl);
}

If we compile this case with the option " -march=rv64gcv -mabi=lp64d  -flto -O0", we will
get the following error:

during RTL pass: expand
../test.c: In function 'foo':
../test.c:5:10: internal compiler error: tree check: expected tree that contains 'typed' structure, have 'ggc_freed' in function_returns_void_p, at config/riscv/riscv-vector-builtins.h:456
    5 |   return __riscv_vadd_vv_i32m1(vs1, vs2, vl);
      |          ^
0x4081948 internal_error(char const*, ...)
        /iothome/jin.ma/code/master/gcc/gcc/diagnostic-global-context.cc:492
0x1dc584d tree_contains_struct_check_failed(tree_node const*, tree_node_structure_enum, char const*, int, char const*)
        /iothome/jin.ma/code/master/gcc/gcc/tree.cc:9177
0x10d8230 contains_struct_check(tree_node*, tree_node_structure_enum, char const*, int, char const*)
        /iothome/jin.ma/code/master/gcc/gcc/tree.h:3779
0x2078f0c riscv_vector::function_call_info::function_returns_void_p()
        /iothome/jin.ma/code/master/gcc/gcc/config/riscv/riscv-vector-builtins.h:456
0x2074f54 riscv_vector::function_expander::function_expander(riscv_vector::function_instance const&, tree_node*, tree_node*, rtx_def*)
        /iothome/jin.ma/code/master/gcc/gcc/config/riscv/riscv-vector-builtins.cc:3920
0x20787b8 riscv_vector::expand_builtin(unsigned int, tree_node*, rtx_def*)
        /iothome/jin.ma/code/master/gcc/gcc/config/riscv/riscv-vector-builtins.cc:4775
0x2029b60 riscv_expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
        /iothome/jin.ma/code/master/gcc/gcc/config/riscv/riscv-builtins.cc:433
0x1167cb7 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
        /iothome/jin.ma/code/master/gcc/gcc/builtins.cc:7763
0x137e5d2 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        /iothome/jin.ma/code/master/gcc/gcc/expr.cc:12390
0x1370068 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        /iothome/jin.ma/code/master/gcc/gcc/expr.cc:9473
0x136434a store_expr(tree_node*, rtx_def*, int, bool, bool)
        /iothome/jin.ma/code/master/gcc/gcc/expr.cc:6766
0x13629e3 expand_assignment(tree_node*, tree_node*, bool)
        /iothome/jin.ma/code/master/gcc/gcc/expr.cc:6487
0x11a8419 expand_call_stmt
        /iothome/jin.ma/code/master/gcc/gcc/cfgexpand.cc:2893
0x11ac48e expand_gimple_stmt_1
        /iothome/jin.ma/code/master/gcc/gcc/cfgexpand.cc:3962
0x11acaad expand_gimple_stmt
        /iothome/jin.ma/code/master/gcc/gcc/cfgexpand.cc:4104
0x11b55a1 expand_gimple_basic_block
        /iothome/jin.ma/code/master/gcc/gcc/cfgexpand.cc:6160
0x11b7b96 execute
        /iothome/jin.ma/code/master/gcc/gcc/cfgexpand.cc:6899
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
lto-wrapper: fatal error: riscv64-unknown-linux-gnu-gcc returned 1 exit status
compilation terminated.
/mnt/ssd/jin.ma/install/master/bin/../lib/gcc/riscv64-unknown-linux-gnu/15.0.0/../../../../riscv64-unknown-linux-gnu/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status

This patch tried to fix it, but it didn't fix it completely. If we use "-fchecking", we still
have ICE. This has to do with function ggc_grow in the LTO phase. Maybe the fix for this ICE
is that we use something like "malloc" instead of "ggc_alloc" for "registered_functions",
I'm not sure there is a better way.

If this is a problem and needs to be fixed, I am happy to try to solve it.
Li, Pan2 Sept. 7, 2024, 10:38 a.m. UTC | #3
> #include <riscv_vector.h>
> 
> vint32m1_t foo(vint32m1_t vs1, vint32m1_t vs2, size_t vl) 
> {
>   return __riscv_vadd_vv_i32m1(vs1, vs2, vl);
> }

To double confirm, you mean "riscv64-linux-gnu-gcc-14 -march=rv64gcv -mabi=lp64d -flto -O0 tmp.c -c -S -o -" with above is able to reproduce this ICE?

Pan

-----Original Message-----
From: Jin Ma <jinma@linux.alibaba.com> 
Sent: Saturday, September 7, 2024 5:43 PM
To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
Cc: jeffreyalaw@gmail.com; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; jinma.contrib@gmail.com
Subject: Re: [PATCH] RISC-V: Fix ICE for rvv in lto

> > +/* 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;
> > +}
> 
> Interesting, do we still have ice when there is no __voltaile__ for vd? As well as gcc-14 branch.
> Because it is quite a common case that should be covered by test already.
> 
> Pan

Yes, I am also surprised that this kind of ICE will appear. It really should be covered by
test cases. But in fact, if we do not use zvfh or zvfhmin in arch, rvv cannot be used in LTO.

This has nothing to do with "__voltaile__". "__voltaile__" in the case is just that I want it
to be compiled to the end and not optimized.

In fact, a simple case can reproduce ICE, including gcc-14 and master, for example:

#include <riscv_vector.h>

vint32m1_t foo(vint32m1_t vs1, vint32m1_t vs2, size_t vl) 
{
  return __riscv_vadd_vv_i32m1(vs1, vs2, vl);
}

If we compile this case with the option " -march=rv64gcv -mabi=lp64d  -flto -O0", we will
get the following error:

during RTL pass: expand
../test.c: In function 'foo':
../test.c:5:10: internal compiler error: tree check: expected tree that contains 'typed' structure, have 'ggc_freed' in function_returns_void_p, at config/riscv/riscv-vector-builtins.h:456
    5 |   return __riscv_vadd_vv_i32m1(vs1, vs2, vl);
      |          ^
0x4081948 internal_error(char const*, ...)
        /iothome/jin.ma/code/master/gcc/gcc/diagnostic-global-context.cc:492
0x1dc584d tree_contains_struct_check_failed(tree_node const*, tree_node_structure_enum, char const*, int, char const*)
        /iothome/jin.ma/code/master/gcc/gcc/tree.cc:9177
0x10d8230 contains_struct_check(tree_node*, tree_node_structure_enum, char const*, int, char const*)
        /iothome/jin.ma/code/master/gcc/gcc/tree.h:3779
0x2078f0c riscv_vector::function_call_info::function_returns_void_p()
        /iothome/jin.ma/code/master/gcc/gcc/config/riscv/riscv-vector-builtins.h:456
0x2074f54 riscv_vector::function_expander::function_expander(riscv_vector::function_instance const&, tree_node*, tree_node*, rtx_def*)
        /iothome/jin.ma/code/master/gcc/gcc/config/riscv/riscv-vector-builtins.cc:3920
0x20787b8 riscv_vector::expand_builtin(unsigned int, tree_node*, rtx_def*)
        /iothome/jin.ma/code/master/gcc/gcc/config/riscv/riscv-vector-builtins.cc:4775
0x2029b60 riscv_expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
        /iothome/jin.ma/code/master/gcc/gcc/config/riscv/riscv-builtins.cc:433
0x1167cb7 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
        /iothome/jin.ma/code/master/gcc/gcc/builtins.cc:7763
0x137e5d2 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        /iothome/jin.ma/code/master/gcc/gcc/expr.cc:12390
0x1370068 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        /iothome/jin.ma/code/master/gcc/gcc/expr.cc:9473
0x136434a store_expr(tree_node*, rtx_def*, int, bool, bool)
        /iothome/jin.ma/code/master/gcc/gcc/expr.cc:6766
0x13629e3 expand_assignment(tree_node*, tree_node*, bool)
        /iothome/jin.ma/code/master/gcc/gcc/expr.cc:6487
0x11a8419 expand_call_stmt
        /iothome/jin.ma/code/master/gcc/gcc/cfgexpand.cc:2893
0x11ac48e expand_gimple_stmt_1
        /iothome/jin.ma/code/master/gcc/gcc/cfgexpand.cc:3962
0x11acaad expand_gimple_stmt
        /iothome/jin.ma/code/master/gcc/gcc/cfgexpand.cc:4104
0x11b55a1 expand_gimple_basic_block
        /iothome/jin.ma/code/master/gcc/gcc/cfgexpand.cc:6160
0x11b7b96 execute
        /iothome/jin.ma/code/master/gcc/gcc/cfgexpand.cc:6899
Please submit a full bug report, with preprocessed source (by using -freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
lto-wrapper: fatal error: riscv64-unknown-linux-gnu-gcc returned 1 exit status
compilation terminated.
/mnt/ssd/jin.ma/install/master/bin/../lib/gcc/riscv64-unknown-linux-gnu/15.0.0/../../../../riscv64-unknown-linux-gnu/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status

This patch tried to fix it, but it didn't fix it completely. If we use "-fchecking", we still
have ICE. This has to do with function ggc_grow in the LTO phase. Maybe the fix for this ICE
is that we use something like "malloc" instead of "ggc_alloc" for "registered_functions",
I'm not sure there is a better way.

If this is a problem and needs to be fixed, I am happy to try to solve it.
Jeff Law Sept. 7, 2024, 5:25 p.m. UTC | #4
On 9/6/24 11:30 AM, Jin Ma 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?
In general allocating things with the collector API is safe.

But it's ultimately a garbage collector, so if the object is not 
reachable via the registered GC roots, then it'll get collected.  This 
is the most common issue that folks run into.

jeff
Jin Ma Sept. 8, 2024, 5:15 a.m. UTC | #5
> > #include <riscv_vector.h>
> > 
> > vint32m1_t foo(vint32m1_t vs1, vint32m1_t vs2, size_t vl) 
> > {
> >   return __riscv_vadd_vv_i32m1(vs1, vs2, vl);
> > }
> 
> To double confirm, you mean "riscv64-linux-gnu-gcc-14 -march=rv64gcv -mabi=lp64d -flto -O0 tmp.c -c -S -o -" with above is able to reproduce this ICE?
> 
> Pan

Not too accurate, please don't add "-S" or "-c", let the compilation go to the linker and try to generate the binary.
The normal result of compilation should be to throw an error that the main function cannot be found, but unfortunately
ICE appears.

By the way, The gcc-14 in my environment is built on releases/gcc-14, I didn't download any compiled gcc.

Of course, it is also possible that my local environment is broken, and I will check it again.

BR
Jin
Li, Pan2 Sept. 8, 2024, 11:39 a.m. UTC | #6
I see, I can reproduce this when build "-march=rv64gcv -mabi=lp64d -flto -O0 test.c -o test.elf".

#include <riscv_vector.h>

int
main ()
{
  size_t vl = 8;
  vint32m1_t vs1 = {};
  vint32m1_t vs2 = {};
  vint32m1_t vd = __riscv_vadd_vv_i32m1(vs1, vs2, vl);

  return (int)&vd;
}

Pan

-----Original Message-----
From: Jin Ma <jinma@linux.alibaba.com> 
Sent: Sunday, September 8, 2024 1:15 PM
To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
Cc: jeffreyalaw@gmail.com; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; jinma.contrib@gmail.com
Subject: Re: [PATCH] RISC-V: Fix ICE for rvv in lto

> > #include <riscv_vector.h>
> > 
> > vint32m1_t foo(vint32m1_t vs1, vint32m1_t vs2, size_t vl) 
> > {
> >   return __riscv_vadd_vv_i32m1(vs1, vs2, vl);
> > }
> 
> To double confirm, you mean "riscv64-linux-gnu-gcc-14 -march=rv64gcv -mabi=lp64d -flto -O0 tmp.c -c -S -o -" with above is able to reproduce this ICE?
> 
> Pan

Not too accurate, please don't add "-S" or "-c", let the compilation go to the linker and try to generate the binary.
The normal result of compilation should be to throw an error that the main function cannot be found, but unfortunately
ICE appears.

By the way, The gcc-14 in my environment is built on releases/gcc-14, I didn't download any compiled gcc.

Of course, it is also possible that my local environment is broken, and I will check it again.

BR
Jin
Richard Biener Sept. 9, 2024, 7:15 a.m. UTC | #7
On Fri, Sep 6, 2024 at 7:31 PM Jin Ma <jinma@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
>
Jin Ma Sept. 9, 2024, 8 a.m. UTC | #8
> > 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.

Very helpful guide, I will try to fix it.

BR,
Jin
Jin Ma Sept. 9, 2024, 10:30 a.m. UTC | #9
> I see, I can reproduce this when build "-march=rv64gcv -mabi=lp64d -flto -O0 test.c -o test.elf".
> 
> #include <riscv_vector.h>
> 
> int
> main ()
> {
>   size_t vl = 8;
>   vint32m1_t vs1 = {};
>   vint32m1_t vs2 = {};
>   vint32m1_t vd = __riscv_vadd_vv_i32m1(vs1, vs2, vl);
> 
>   return (int)&vd;
> }
> 
> Pan

Hi, Pan

Any comments on this patch?

I think this patch is quite important, because RVV is completely unavailable on LTO at
present. In fact, I discovered this ICE while trying to compile some computational
libraries using LTO. Unfortunately, none of the libraries currently compile through
properly.

BR
Jin
Li, Pan2 Sept. 9, 2024, 10:44 p.m. UTC | #10
> Any comments on this patch?

I may need some time to go through all details (PS: Sorry I cannot approve patches, leave it to juzhe or kito).
Thanks a lot for fixing this.

Pan

-----Original Message-----
From: Jin Ma <jinma@linux.alibaba.com> 
Sent: Monday, September 9, 2024 6:30 PM
To: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org
Cc: jeffreyalaw@gmail.com; juzhe.zhong@rivai.ai; kito.cheng@gmail.com; jinma.contrib@gmail.com
Subject: Re: [PATCH] RISC-V: Fix ICE for rvv in lto

> I see, I can reproduce this when build "-march=rv64gcv -mabi=lp64d -flto -O0 test.c -o test.elf".
> 
> #include <riscv_vector.h>
> 
> int
> main ()
> {
>   size_t vl = 8;
>   vint32m1_t vs1 = {};
>   vint32m1_t vs2 = {};
>   vint32m1_t vd = __riscv_vadd_vv_i32m1(vs1, vs2, vl);
> 
>   return (int)&vd;
> }
> 
> Pan

Hi, Pan

Any comments on this patch?

I think this patch is quite important, because RVV is completely unavailable on LTO at
present. In fact, I discovered this ICE while trying to compile some computational
libraries using LTO. Unfortunately, none of the libraries currently compile through
properly.

BR
Jin
diff mbox series

Patch

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;
+}