LGTM, thanks :)
On Fri, Mar 22, 2024 at 9:13 AM <pan2...@intel.com> wrote:
>
> From: Pan Li <pan2...@intel.com>
>
> This patch would like to fix one ICE for __attribute__((target("arch=+v"))
> and likewise extension(s). Given we have sample code as below:
>
> void __attribute__((target("arch=+v")))
> test_2 (int *a, int *b, int *out, unsigned count)
> {
> unsigned i;
> for (i = 0; i < count; i++)
> out[i] = a[i] + b[i];
> }
>
> It will have ICE when build with -march=rv64gc -O3.
>
> test.c: In function ‘test_2’:
> test.c:4:1: internal compiler error: Floating point exception
> 4 | {
> | ^
> 0x1a5891b crash_signal
> .../__RISC-V_BUILD__/../gcc/toplev.cc:319
> 0x7f0a7884251f ???
> ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
> 0x1f51ba4 riscv_hard_regno_nregs
> .../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:8143
> 0x1967bb9 init_reg_modes_target()
> .../__RISC-V_BUILD__/../gcc/reginfo.cc:471
> 0x13fc029 init_emit_regs()
> .../__RISC-V_BUILD__/../gcc/emit-rtl.cc:6237
> 0x1a5b83d target_reinit()
> .../__RISC-V_BUILD__/../gcc/toplev.cc:1936
> 0x35e374d save_target_globals()
> .../__RISC-V_BUILD__/../gcc/target-globals.cc:92
> 0x35e381f save_target_globals_default_opts()
> .../__RISC-V_BUILD__/../gcc/target-globals.cc:122
> 0x1f544cc riscv_save_restore_target_globals(tree_node*)
> .../__RISC-V_BUILD__/../gcc/config/riscv/riscv.cc:9138
> 0x1f55c36 riscv_set_current_function
> ...
>
> There are two reasons for this ICE.
> 1. The implied extension(s) of v are not well handled and the
> TARGET_MIN_VLEN is 0 which is not reinitialized. Then the
> size / TARGET_MIN_VLEN will have DivideByZero.
> 2. The machine modes of the vector types will be vary after
> the v extension is introduced.
>
> This patch passed below testsuite:
> 1. The riscv fully regression test.
>
> PR target/114352
>
> gcc/ChangeLog:
>
> * common/config/riscv/riscv-common.cc (riscv_subset_list::parse):
> Replace implied, combine and check to func finalize.
> (riscv_subset_list::finalize): New func impl to take care of
> implied, combine ext and related checks.
> * config/riscv/riscv-subset.h: Add func decl for finalize.
> * config/riscv/riscv-target-attr.cc
> (riscv_target_attr_parser::parse_arch):
> Finalize the ext before return succeed.
> * config/riscv/riscv.cc (riscv_set_current_function): Reinit the
> machine mode before when set cur function.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/base/pr114352-1.c: New test.
> * gcc.target/riscv/rvv/base/pr114352-2.c: New test.
>
> Signed-off-by: Pan Li <pan2...@intel.com>
> ---
> gcc/common/config/riscv/riscv-common.cc | 31 ++++++----
> gcc/config/riscv/riscv-subset.h | 2 +
> gcc/config/riscv/riscv-target-attr.cc | 2 +
> gcc/config/riscv/riscv.cc | 4 ++
> .../gcc.target/riscv/rvv/base/pr114352-1.c | 58 +++++++++++++++++++
> .../gcc.target/riscv/rvv/base/pr114352-2.c | 27 +++++++++
> 6 files changed, 114 insertions(+), 10 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
>
> diff --git a/gcc/common/config/riscv/riscv-common.cc
> b/gcc/common/config/riscv/riscv-common.cc
> index 440127a2af0..15d44245b3c 100644
> --- a/gcc/common/config/riscv/riscv-common.cc
> +++ b/gcc/common/config/riscv/riscv-common.cc
> @@ -1428,16 +1428,7 @@ riscv_subset_list::parse (const char *arch, location_t
> loc)
> if (p == NULL)
> goto fail;
>
> - for (itr = subset_list->m_head; itr != NULL; itr = itr->next)
> - {
> - subset_list->handle_implied_ext (itr->name.c_str ());
> - }
> -
> - /* Make sure all implied extensions are included. */
> - gcc_assert (subset_list->check_implied_ext ());
> -
> - subset_list->handle_combine_ext ();
> - subset_list->check_conflict_ext ();
> + subset_list->finalize ();
>
> return subset_list;
>
> @@ -1467,6 +1458,26 @@ riscv_subset_list::set_loc (location_t loc)
> m_loc = loc;
> }
>
> +/* Make sure the implied or combined extension is included after add
> + a new std extension to subset list or likewise. For exmaple as below,
> +
> + void __attribute__((target("arch=+v"))) func () with -march=rv64gc.
> +
> + The implied zvl128b and zve64d of the std v should be included. */
> +void
> +riscv_subset_list::finalize ()
> +{
> + riscv_subset_t *subset;
> +
> + for (subset = m_head; subset != NULL; subset = subset->next)
> + handle_implied_ext (subset->name.c_str ());
> +
> + gcc_assert (check_implied_ext ());
> +
> + handle_combine_ext ();
> + check_conflict_ext ();
> +}
> +
> /* Return the current arch string. */
>
> std::string
> diff --git a/gcc/config/riscv/riscv-subset.h b/gcc/config/riscv/riscv-subset.h
> index ae849e2a302..ec979040e8c 100644
> --- a/gcc/config/riscv/riscv-subset.h
> +++ b/gcc/config/riscv/riscv-subset.h
> @@ -105,6 +105,8 @@ public:
> int match_score (riscv_subset_list *) const;
>
> void set_loc (location_t);
> +
> + void finalize ();
> };
>
> extern const riscv_subset_list *riscv_current_subset_list (void);
> diff --git a/gcc/config/riscv/riscv-target-attr.cc
> b/gcc/config/riscv/riscv-target-attr.cc
> index 9dbb78f28cc..37645adbb20 100644
> --- a/gcc/config/riscv/riscv-target-attr.cc
> +++ b/gcc/config/riscv/riscv-target-attr.cc
> @@ -136,6 +136,8 @@ riscv_target_attr_parser::parse_arch (const char *str)
> }
> token = strtok_r (NULL, ",", &str_to_check);
> }
> +
> + m_subset_list->finalize ();
> return true;
> }
> fail:
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 02a927f96b4..742974aec33 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -9492,6 +9492,10 @@ riscv_set_current_function (tree decl)
> cl_target_option_restore (&global_options, &global_options_set,
> TREE_TARGET_OPTION (new_tree));
>
> + /* The ISA extension can vary based on the function extension like target.
> + Thus, make sure that the machine modes are reflected correctly here. */
> + init_adjust_machine_modes ();
> +
> riscv_save_restore_target_globals (new_tree);
> }
>
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
> b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
> new file mode 100644
> index 00000000000..b3f1f20fb79
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-1.c
> @@ -0,0 +1,58 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64 -O3 -fno-schedule-insns
> -fno-schedule-insns2" } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_1:
> +** beq\s+a3,\s*zero,\s*\.L[0-9]+
> +** ...
> +** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
> +** ...
> +** ret
> +*/
> +void
> +test_1 (int *a, int *b, int *out, unsigned count)
> +{
> + unsigned i;
> +
> + for (i = 0; i < count; i++)
> + out[i] = a[i] + b[i];
> +}
> +
> +/*
> +** test_2:
> +** ...
> +** vsetvli\s+[atx][0-9]+,\s*[atx][0-9]+,\s*e32,\s*m1,\s*ta,\s*ma
> +** ...
> +** vadd\.vv\s+v[0-9]+,\s*v[0-9]+,\s*v[0-9]+
> +** ...
> +*/
> +void
> +__attribute__((target("arch=+v")))
> +test_2 (int *a, int *b, int *out, unsigned count)
> +{
> + unsigned i;
> +
> + for (i = 0; i < count; i++)
> + out[i] = a[i] + b[i];
> +}
> +
> +/*
> +** test_3:
> +** beq\s+a3,\s*zero,\s*\.L[0-9]+
> +** ...
> +** bne\s+[atx][0-9]+,\s*[atx][0-9]+,\s*\.L[0-9]+
> +** ...
> +** ret
> +*/
> +void
> +test_3 (int *a, int *b, int *out, unsigned count)
> +{
> + unsigned i;
> +
> + for (i = 0; i < count; i++)
> + out[i] = a[i] + b[i];
> +}
> +
> +/* { dg-final { scan-assembler ".attribute arch,
> \"rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0\"" } } */
> +/* { dg-final { scan-assembler ".option arch,
> rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_v1p0_zicsr2p0_zifencei2p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl128b1p0_zvl32b1p0_zvl64b1p0"
> } } */
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
> b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
> new file mode 100644
> index 00000000000..3b3d69d2751
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114352-2.c
> @@ -0,0 +1,27 @@
> +/* Test that we do not have ice when compile */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc -mabi=lp64 -O3" } */
> +
> +#define DEF_ATTR_FUNC(ATTR, ID) \
> +void ATTR \
> +test_##ID (int *a, int *b, int *out, unsigned count) \
> +{ \
> + unsigned i; \
> + \
> + for (i = 0; i < count; i++) \
> + out[i] = a[i] + b[i]; \
> +}
> +
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve32x"))), 1)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve32f"))), 2)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve64x"))), 3)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve64f"))), 4)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zve64d"))), 5)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+v"))), 6)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl64b"))), 7)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl128b"))), 8)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl256b"))), 9)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl512b"))), 10)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl1024b"))), 11)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl2048b"))), 12)
> +DEF_ATTR_FUNC (__attribute__((target("arch=+zvl4096b"))), 13)
> --
> 2.34.1
>
