diff mbox series

[v2,2/2] RISC-V: Fix ICE due to inconsistency of RVV intrinsic list in lto and cc1.

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

Commit Message

Jin Ma Sept. 10, 2024, 5:56 a.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.

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-11.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 ++++++++++++++++++++++-
 3 files changed, 96 insertions(+), 70 deletions(-)

Comments

Li, Pan2 Sept. 11, 2024, 12:58 a.m. UTC | #1
> * gcc.target/riscv/rvv/base/bug-11.c: New test.

Seems you missed this file in patch v2?

> +/* 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 ();
> +}

Looks this part almost the same as most of riscv_pragma_intrinsic except register_builtin_types ().
I wonder if we can wrap a helper to avoid code duplication, and IMO the _lto suffix should be
removed as the body of function has nothing to do with lto.

Otherwise no comments from myside, and l'd leave it to kito or juzhe.

Pan

-----Original Message-----
From: Jin Ma <jinma@linux.alibaba.com> 
Sent: Tuesday, September 10, 2024 1:57 PM
To: gcc-patches@gcc.gnu.org
Cc: jeffreyalaw@gmail.com; juzhe.zhong@rivai.ai; Li, Pan2 <pan2.li@intel.com>; kito.cheng@gmail.com; richard.guenther@gmail.com; jinma.contrib@gmail.com; Jin Ma <jinma@linux.alibaba.com>
Subject: [PATCH v2 2/2] RISC-V: Fix ICE due to inconsistency of RVV intrinsic list in lto and cc1.

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.

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-11.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 ++++++++++++++++++++++-
 3 files changed, 96 insertions(+), 70 deletions(-)

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 0176670fbdf2..421c40be3ba5 100644
--- a/gcc/config/riscv/riscv-vector-builtins.cc
+++ b/gcc/config/riscv/riscv-vector-builtins.cc
@@ -4510,6 +4510,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
@@ -4518,9 +4595,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 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 0176670fbdf2..421c40be3ba5 100644
--- a/gcc/config/riscv/riscv-vector-builtins.cc
+++ b/gcc/config/riscv/riscv-vector-builtins.cc
@@ -4510,6 +4510,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
@@ -4518,9 +4595,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