On Mon, Feb 26, 2024 at 3:22 PM <pan2...@intel.com> wrote:
>
> From: Pan Li <pan2...@intel.com>
>
> We allowed vector type for get_stored_val when read is less than or
> equal to store in previous. Unfortunately, we missed to adjust the
> validate_subreg part accordingly. When the vector type's size is
> less than vector register, it will be considered as invalid in the
> validate_subreg.
>
> Consider the validate_subreg is kind of a can with worms and we are
> in stage 4. We will fix the issue from the DES side, and make sure
> the subreg is valid for both the read_mode and store_mode before
> perform the real gen_lowpart.
>
> The below test are passed for this patch:
>
> * The x86 bootstrap test.
> * The x86 regression test.
> * The riscv regression test.
> * The aarch64 regression test.
>
> gcc/ChangeLog:
>
> * dse.cc (get_stored_val): Add validate_subreg check before
> perform the gen_lowpart for rtl.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/ssa-fre-44.c: Add compile option to trigger
> the ICE.
> * gcc.target/riscv/rvv/base/bug-6.c: New test.
>
> Signed-off-by: Pan Li <pan2...@intel.com>
> ---
> gcc/dse.cc | 4 +++-
> gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-44.c | 2 +-
> .../gcc.target/riscv/rvv/base/bug-6.c | 22 +++++++++++++++++++
> 3 files changed, 26 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
>
> diff --git a/gcc/dse.cc b/gcc/dse.cc
> index edc7a1dfecf..1596da91da0 100644
> --- a/gcc/dse.cc
> +++ b/gcc/dse.cc
> @@ -1946,7 +1946,9 @@ get_stored_val (store_info *store_info, machine_mode
> read_mode,
> copy_rtx (store_info->const_rhs));
> else if (VECTOR_MODE_P (read_mode) && VECTOR_MODE_P (store_mode)
> && known_le (GET_MODE_BITSIZE (read_mode), GET_MODE_BITSIZE (store_mode))
> - && targetm.modes_tieable_p (read_mode, store_mode))
> + && targetm.modes_tieable_p (read_mode, store_mode)
> + && validate_subreg (read_mode, store_mode, copy_rtx (store_info->rhs),
> + subreg_lowpart_offset (read_mode, store_mode)))
> read_reg = gen_lowpart (read_mode, copy_rtx (store_info->rhs));
Thanks for the 2nd try. I'll note the above uses gen_lowpart but
validate_subreg
which is sort-of a mismatch? But I'll leave this for review to people with more
knowledge in this area. Jeff? Richard?
Thanks,
Richard.
> else
> read_reg = extract_low_bits (read_mode, store_mode,
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-44.c
> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-44.c
> index f79b4c142ae..624a00a4f32 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-44.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-44.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-O -fdump-tree-fre1" } */
> +/* { dg-options "-O -fdump-tree-fre1 -O3 -ftree-vectorize" } */
>
> struct A { float x, y; };
> struct B { struct A u; };
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
> b/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
> new file mode 100644
> index 00000000000..5bb00b8f587
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/bug-6.c
> @@ -0,0 +1,22 @@
> +/* Test that we do not have ice when compile */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -ftree-vectorize" } */
> +
> +struct A { float x, y; };
> +struct B { struct A u; };
> +
> +extern void bar (struct A *);
> +
> +float
> +f3 (struct B *x, int y)
> +{
> + struct A p = {1.0f, 2.0f};
> + struct A *q = &x[y].u;
> +
> + __builtin_memcpy (&q->x, &p.x, sizeof (float));
> + __builtin_memcpy (&q->y, &p.y, sizeof (float));
> +
> + bar (&p);
> +
> + return x[y].u.x + x[y].u.y;
> +}
> --
> 2.34.1
>
