Re: [PATCH] aarch64: Fix ICE in poly-int.h due to SLP.
Richard Ball 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
> Sent: 30 January 2024 17:36
> To: Richard Ball
> Cc: gcc-patches@gcc.gnu.org ; Richard Sandiford
> ; Kyrylo Tkachov ; Richard
> Earnshaw ; Marcus Shawcroft
>
> Subject: Re: [PATCH] aarch64: Fix ICE in poly-int.h due to SLP.
>
> On Tue, 30 Jan 2024 at 20:13, Richard Ball 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 ())
> 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.
Re: [PATCH] aarch64: Fix ICE in poly-int.h due to SLP.
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
From: Prathamesh Kulkarni
Sent: 30 January 2024 17:36
To: Richard Ball
Cc: gcc-patches@gcc.gnu.org ; Richard Sandiford
; Kyrylo Tkachov ; Richard
Earnshaw ; Marcus Shawcroft
Subject: Re: [PATCH] aarch64: Fix ICE in poly-int.h due to SLP.
On Tue, 30 Jan 2024 at 20:13, Richard Ball 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 ())
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.
Re: [PATCH] aarch64: Fix ICE in poly-int.h due to SLP.
On Tue, 30 Jan 2024 at 20:13, Richard Ball 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 ())
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.
[PATCH] aarch64: Fix ICE in poly-int.h due to SLP.
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.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 ())
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 (vinfo))
if (!LOOP_VINFO_VECT_FACTOR (linfo).is_constant ())
