| Message ID | 97d79acf-eaac-4c42-9478-4714f79f12ef@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | aarch64: Fix ICE in poly-int.h due to SLP. | expand |
On Tue, 30 Jan 2024 at 20:13, Richard Ball <richard.ball@arm.com> wrote: > > Adds a check to ensure that the input vector arguments > to a function are not variable length. Previously, only the > output vector of a function was checked. Hi, Quoting from patch: @@ -8989,6 +8989,14 @@ vectorizable_slp_permutation_1 (vec_info *vinfo, gimple_stmt_iterator *gsi, instead of relying on the pattern described above. */ if (!nunits.is_constant (&npatterns)) return -1; + FOR_EACH_VEC_ELT (children, i, child) + if (SLP_TREE_VECTYPE (child)) + { + tree child_vectype = SLP_TREE_VECTYPE (child); + poly_uint64 child_nunits = TYPE_VECTOR_SUBPARTS (child_vectype); + if (!child_nunits.is_constant ()) + return -1; + } Just wondering if that'd be equivalent to checking: if (!TYPE_VECTOR_SUBPARTS (op_vectype).is_constant ()) return -1; Instead of (again) iterating over children since we bail out in the function above, if SLP_TREE_VECTYPE (child) and op_vectype are not compatible types ? Also, could you please include the offending test-case in the patch ? Thanks, Prathamesh > > gcc/ChangeLog: > > * tree-vect-slp.cc (vectorizable_slp_permutation_1): > Add variable-length check for vector input arguments > to a function.
Hi Prathamesh, Thanks for the review, I missed that code up above. I've looking into this and it seems to me at least, that what you have suggested, is equivalent. I'll make the change and repost. Thanks, Richard
Richard Ball <Richard.Ball@arm.com> writes: > Hi Prathamesh, > > Thanks for the review, I missed that code up above. > I've looking into this and it seems to me at least, that what you have > suggested, is equivalent. > I'll make the change and repost. The original patch is OK. Checking in the other loop is too early, because we want to accept variable-length vectors when repeating_p is true. Like Prathamesh says, please add the testcase to the testsuite. Thanks, Richard > > Thanks, > Richard > ------------------------------------------------------------------------------- > From: Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> > Sent: 30 January 2024 17:36 > To: Richard Ball <Richard.Ball@arm.com> > Cc: gcc-patches@gcc.gnu.org <gcc-patches@gcc.gnu.org>; Richard Sandiford > <Richard.Sandiford@arm.com>; Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard > Earnshaw <Richard.Earnshaw@arm.com>; Marcus Shawcroft > <Marcus.Shawcroft@arm.com> > Subject: Re: [PATCH] aarch64: Fix ICE in poly-int.h due to SLP. > > On Tue, 30 Jan 2024 at 20:13, Richard Ball <richard.ball@arm.com> wrote: >> >> Adds a check to ensure that the input vector arguments >> to a function are not variable length. Previously, only the >> output vector of a function was checked. > Hi, > Quoting from patch: > @@ -8989,6 +8989,14 @@ vectorizable_slp_permutation_1 (vec_info > *vinfo, gimple_stmt_iterator *gsi, > instead of relying on the pattern described above. */ > if (!nunits.is_constant (&npatterns)) > return -1; > + FOR_EACH_VEC_ELT (children, i, child) > + if (SLP_TREE_VECTYPE (child)) > + { > + tree child_vectype = SLP_TREE_VECTYPE (child); > + poly_uint64 child_nunits = TYPE_VECTOR_SUBPARTS (child_vectype); > + if (!child_nunits.is_constant ()) > + return -1; > + } > > Just wondering if that'd be equivalent to checking: > if (!TYPE_VECTOR_SUBPARTS (op_vectype).is_constant ()) > return -1; > Instead of (again) iterating over children since we bail out in the > function above, > if SLP_TREE_VECTYPE (child) and op_vectype are not compatible types ? > > Also, could you please include the offending test-case in the patch ? > > Thanks, > Prathamesh > >> >> gcc/ChangeLog: >> >> * tree-vect-slp.cc (vectorizable_slp_permutation_1): >> Add variable-length check for vector input arguments >> to a function.
diff --git a/gcc/tree-vect-slp.cc b/gcc/tree-vect-slp.cc index 086377a9ac08528880c90bee3b1d1e08341a6288..bbd2c51d60baa31ea4169081442e2e9d55055e80 100644 --- a/gcc/tree-vect-slp.cc +++ b/gcc/tree-vect-slp.cc @@ -8989,6 +8989,14 @@ vectorizable_slp_permutation_1 (vec_info *vinfo, gimple_stmt_iterator *gsi, instead of relying on the pattern described above. */ if (!nunits.is_constant (&npatterns)) return -1; + FOR_EACH_VEC_ELT (children, i, child) + if (SLP_TREE_VECTYPE (child)) + { + tree child_vectype = SLP_TREE_VECTYPE (child); + poly_uint64 child_nunits = TYPE_VECTOR_SUBPARTS (child_vectype); + if (!child_nunits.is_constant ()) + return -1; + } nelts_per_pattern = ncopies = 1; if (loop_vec_info linfo = dyn_cast <loop_vec_info> (vinfo)) if (!LOOP_VINFO_VECT_FACTOR (linfo).is_constant (&ncopies))
Adds a check to ensure that the input vector arguments to a function are not variable length. Previously, only the output vector of a function was checked. gcc/ChangeLog: * tree-vect-slp.cc (vectorizable_slp_permutation_1): Add variable-length check for vector input arguments to a function.