On April 14, 2021 5:13:23 PM GMT+02:00, Richard Sandiford via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: >PR99929 is one of those “how did we get away with this for so long” >bugs: the equality routines weren't checking whether two >variable-length >CONST_VECTORs had the same encoding. This meant that: > > { 1, 0, 0, 0, 0, 0, ... } > >would appear to be equal to: > > { 1, 0, 1, 0, 1, 0, ... } > >since both are represented using the elements { 1, 0 }. > >Tested on aarch64-linux-gnu, arm-linux-gnueabihf, armeb-eabi >and x86_64-linux-gnu. OK to install? I'd like to backport >as far as GCC 8, even though the testcase itself requires >GCC 10 or later.
OK. Richard. >Richard > > >gcc/ > PR rtl-optimization/99929 > * rtl.h (same_vector_encodings_p): New function. > * cse.c (exp_equiv_p): Check that CONST_VECTORs have the same >encoding. > * cselib.c (rtx_equal_for_cselib_1): Likewise. > * jump.c (rtx_renumbered_equal_p): Likewise. > * lra-constraints.c (operands_match_p): Likewise. > * reload.c (operands_match_p): Likewise. > * rtl.c (rtx_equal_p_cb, rtx_equal_p): Likewise. > >gcc/testsuite/ > * gcc.target/aarch64/sve/pr99929_1.c: New file. > * gcc.target/aarch64/sve/pr99929_2.c: Likewise. >--- > gcc/cse.c | 5 +++++ > gcc/cselib.c | 5 +++++ > gcc/jump.c | 5 +++++ > gcc/lra-constraints.c | 5 +++++ > gcc/reload.c | 5 +++++ > gcc/rtl.c | 10 ++++++++++ > gcc/rtl.h | 17 +++++++++++++++++ > .../gcc.target/aarch64/sve/pr99929_1.c | 16 ++++++++++++++++ > .../gcc.target/aarch64/sve/pr99929_2.c | 5 +++++ > 9 files changed, 73 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr99929_1.c > create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/pr99929_2.c > >diff --git a/gcc/cse.c b/gcc/cse.c >index 37c6959abea..df191d5aa3f 100644 >--- a/gcc/cse.c >+++ b/gcc/cse.c >@@ -2637,6 +2637,11 @@ exp_equiv_p (const_rtx x, const_rtx y, int >validate, bool for_gcse) > CASE_CONST_UNIQUE: > return x == y; > >+ case CONST_VECTOR: >+ if (!same_vector_encodings_p (x, y)) >+ return false; >+ break; >+ > case LABEL_REF: > return label_ref_label (x) == label_ref_label (y); > >diff --git a/gcc/cselib.c b/gcc/cselib.c >index 2d34a914c6b..779874eeb2d 100644 >--- a/gcc/cselib.c >+++ b/gcc/cselib.c >@@ -1048,6 +1048,11 @@ rtx_equal_for_cselib_1 (rtx x, rtx y, >machine_mode memmode, int depth) > case DEBUG_EXPR: > return 0; > >+ case CONST_VECTOR: >+ if (!same_vector_encodings_p (x, y)) >+ return false; >+ break; >+ > case DEBUG_IMPLICIT_PTR: > return DEBUG_IMPLICIT_PTR_DECL (x) > == DEBUG_IMPLICIT_PTR_DECL (y); >diff --git a/gcc/jump.c b/gcc/jump.c >index 561dbb70d15..67b5c3374a6 100644 >--- a/gcc/jump.c >+++ b/gcc/jump.c >@@ -1777,6 +1777,11 @@ rtx_renumbered_equal_p (const_rtx x, const_rtx >y) > CASE_CONST_UNIQUE: > return 0; > >+ case CONST_VECTOR: >+ if (!same_vector_encodings_p (x, y)) >+ return false; >+ break; >+ > case LABEL_REF: > /* We can't assume nonlocal labels have their following insns yet. */ > if (LABEL_REF_NONLOCAL_P (x) || LABEL_REF_NONLOCAL_P (y)) >diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c >index 62bcfc31772..1560f652da6 100644 >--- a/gcc/lra-constraints.c >+++ b/gcc/lra-constraints.c >@@ -834,6 +834,11 @@ operands_match_p (rtx x, rtx y, int y_hard_regno) > CASE_CONST_UNIQUE: > return false; > >+ case CONST_VECTOR: >+ if (!same_vector_encodings_p (x, y)) >+ return false; >+ break; >+ > case LABEL_REF: > return label_ref_label (x) == label_ref_label (y); > case SYMBOL_REF: >diff --git a/gcc/reload.c b/gcc/reload.c >index 461fd0272eb..e18e27c2405 100644 >--- a/gcc/reload.c >+++ b/gcc/reload.c >@@ -2310,6 +2310,11 @@ operands_match_p (rtx x, rtx y) > CASE_CONST_UNIQUE: > return 0; > >+ case CONST_VECTOR: >+ if (!same_vector_encodings_p (x, y)) >+ return false; >+ break; >+ > case LABEL_REF: > return label_ref_label (x) == label_ref_label (y); > case SYMBOL_REF: >diff --git a/gcc/rtl.c b/gcc/rtl.c >index 1aa794c82ca..e4ae1683069 100644 >--- a/gcc/rtl.c >+++ b/gcc/rtl.c >@@ -466,6 +466,11 @@ rtx_equal_p_cb (const_rtx x, const_rtx y, >rtx_equal_p_callback_function cb) > CASE_CONST_UNIQUE: > return 0; > >+ case CONST_VECTOR: >+ if (!same_vector_encodings_p (x, y)) >+ return false; >+ break; >+ > case DEBUG_IMPLICIT_PTR: > return DEBUG_IMPLICIT_PTR_DECL (x) > == DEBUG_IMPLICIT_PTR_DECL (y); >@@ -608,6 +613,11 @@ rtx_equal_p (const_rtx x, const_rtx y) > CASE_CONST_UNIQUE: > return 0; > >+ case CONST_VECTOR: >+ if (!same_vector_encodings_p (x, y)) >+ return false; >+ break; >+ > case DEBUG_IMPLICIT_PTR: > return DEBUG_IMPLICIT_PTR_DECL (x) > == DEBUG_IMPLICIT_PTR_DECL (y); >diff --git a/gcc/rtl.h b/gcc/rtl.h >index a392721c8d0..398d745aff5 100644 >--- a/gcc/rtl.h >+++ b/gcc/rtl.h >@@ -3087,6 +3087,23 @@ vec_series_p (const_rtx x, rtx *base_out, rtx >*step_out) > return const_vec_series_p (x, base_out, step_out); > } > >+/* Return true if CONST_VECTORs X and Y, which are known to have the >same mode, >+ also have the same encoding. This means that they are equal >whenever their >+ operands are equal. */ >+ >+inline bool >+same_vector_encodings_p (const_rtx x, const_rtx y) >+{ >+ /* Don't be fussy about the encoding of constant-length vectors, >+ since XVECEXP (X, 0) and XVECEXP (Y, 0) list all the elements >anyway. */ >+ if (poly_uint64 (CONST_VECTOR_NUNITS (x)).is_constant ()) >+ return true; >+ >+ return (CONST_VECTOR_NPATTERNS (x) == CONST_VECTOR_NPATTERNS (y) >+ && (CONST_VECTOR_NELTS_PER_PATTERN (x) >+ == CONST_VECTOR_NELTS_PER_PATTERN (y))); >+} >+ >/* Return the unpromoted (outer) mode of SUBREG_PROMOTED_VAR_P subreg >X. */ > > inline scalar_int_mode >diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr99929_1.c >b/gcc/testsuite/gcc.target/aarch64/sve/pr99929_1.c >new file mode 100644 >index 00000000000..1fe18136e28 >--- /dev/null >+++ b/gcc/testsuite/gcc.target/aarch64/sve/pr99929_1.c >@@ -0,0 +1,16 @@ >+/* { dg-do run { target aarch64_sve_hw } } */ >+/* { dg-options "-O2 -ftree-vectorize" } */ >+ >+#include <arm_sve.h> >+ >+static void e(short *g, short p2) { *g ^= p2; } >+static short m[23]; >+int main() { >+ for (unsigned i = 0; i < 23; ++i) >+ m[i] = 4; >+ if (svaddv(svptrue_pat_b32(SV_VL1), svdup_u32(1)) != 1) >+ __builtin_abort(); >+ for (unsigned i = 0; i < 3; ++i) >+ e(m, m[i]); >+ return 0; >+} >diff --git a/gcc/testsuite/gcc.target/aarch64/sve/pr99929_2.c >b/gcc/testsuite/gcc.target/aarch64/sve/pr99929_2.c >new file mode 100644 >index 00000000000..50d432db9b8 >--- /dev/null >+++ b/gcc/testsuite/gcc.target/aarch64/sve/pr99929_2.c >@@ -0,0 +1,5 @@ >+/* { dg-options "-O2 -ftree-vectorize" } */ >+ >+#include "pr99929_1.c" >+ >+/* { dg-final { scan-assembler {\tptrue\tp[0-7].[bhsd], vl1\n} } } */