From: Richard Sandiford <richard.sandiford@arm.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>,
gcc Patches <gcc-patches@gcc.gnu.org>
Subject: Re: ICE after folding svld1rq to vec_perm_expr duing forwprop
Date: Thu, 14 Jul 2022 12:52:38 +0100 [thread overview]
Message-ID: <mpt5yjzx661.fsf@arm.com> (raw)
In-Reply-To: <CAFiYyc2KBAxGcKAy9yVV=r5DU2_ZC_osWxka2daq9A10fr-0sA@mail.gmail.com> (Richard Biener's message of "Thu, 14 Jul 2022 10:33:52 +0200")
Richard Biener <richard.guenther@gmail.com> writes:
> On Thu, Jul 14, 2022 at 9:55 AM Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> wrote:
>>
>> On Wed, 13 Jul 2022 at 12:22, Richard Biener <richard.guenther@gmail.com> wrote:
>> >
>> > On Tue, Jul 12, 2022 at 9:12 PM Prathamesh Kulkarni via Gcc-patches
>> > <gcc-patches@gcc.gnu.org> wrote:
>> > >
>> > > Hi Richard,
>> > > For the following test:
>> > >
>> > > svint32_t f2(int a, int b, int c, int d)
>> > > {
>> > > int32x4_t v = (int32x4_t) {a, b, c, d};
>> > > return svld1rq_s32 (svptrue_b8 (), &v[0]);
>> > > }
>> > >
>> > > The compiler emits following ICE with -O3 -mcpu=generic+sve:
>> > > foo.c: In function ‘f2’:
>> > > foo.c:4:11: error: non-trivial conversion in ‘view_convert_expr’
>> > > 4 | svint32_t f2(int a, int b, int c, int d)
>> > > | ^~
>> > > svint32_t
>> > > __Int32x4_t
>> > > _7 = VIEW_CONVERT_EXPR<__Int32x4_t>(_8);
>> > > during GIMPLE pass: forwprop
>> > > dump file: foo.c.109t.forwprop2
>> > > foo.c:4:11: internal compiler error: verify_gimple failed
>> > > 0xfda04a verify_gimple_in_cfg(function*, bool)
>> > > ../../gcc/gcc/tree-cfg.cc:5568
>> > > 0xe9371f execute_function_todo
>> > > ../../gcc/gcc/passes.cc:2091
>> > > 0xe93ccb execute_todo
>> > > ../../gcc/gcc/passes.cc:2145
>> > >
>> > > This happens because, after folding svld1rq_s32 to vec_perm_expr, we have:
>> > > int32x4_t v;
>> > > __Int32x4_t _1;
>> > > svint32_t _9;
>> > > vector(4) int _11;
>> > >
>> > > <bb 2> :
>> > > _1 = {a_3(D), b_4(D), c_5(D), d_6(D)};
>> > > v_12 = _1;
>> > > _11 = v_12;
>> > > _9 = VEC_PERM_EXPR <_11, _11, { 0, 1, 2, 3, ... }>;
>> > > return _9;
>> > >
>> > > During forwprop, simplify_permutation simplifies vec_perm_expr to
>> > > view_convert_expr,
>> > > and the end result becomes:
>> > > svint32_t _7;
>> > > __Int32x4_t _8;
>> > >
>> > > ;; basic block 2, loop depth 0
>> > > ;; pred: ENTRY
>> > > _8 = {a_2(D), b_3(D), c_4(D), d_5(D)};
>> > > _7 = VIEW_CONVERT_EXPR<__Int32x4_t>(_8);
>> > > return _7;
>> > > ;; succ: EXIT
>> > >
>> > > which causes the error duing verify_gimple since VIEW_CONVERT_EXPR
>> > > has incompatible types (svint32_t, int32x4_t).
>> > >
>> > > The attached patch disables simplification of VEC_PERM_EXPR
>> > > in simplify_permutation, if lhs and rhs have non compatible types,
>> > > which resolves ICE, but am not sure if it's the correct approach ?
>> >
>> > It for sure papers over the issue. I think the error happens earlier,
>> > the V_C_E should have been built with the type of the VEC_PERM_EXPR
>> > which is the type of the LHS. But then you probably run into the
>> > different sizes ICE (VLA vs constant size). I think for this case you
>> > want a BIT_FIELD_REF instead of a VIEW_CONVERT_EXPR,
>> > selecting the "low" part of the VLA vector.
>> Hi Richard,
>> Sorry I don't quite follow. In this case, we use VEC_PERM_EXPR to
>> represent dup operation
>> from fixed width to VLA vector. I am not sure how folding it to
>> BIT_FIELD_REF will work.
>> Could you please elaborate ?
>>
>> Also, the issue doesn't seem restricted to this case.
>> The following test case also ICE's during forwprop:
>> svint32_t foo()
>> {
>> int32x4_t v = (int32x4_t) {1, 2, 3, 4};
>> svint32_t v2 = svld1rq_s32 (svptrue_b8 (), &v[0]);
>> return v2;
>> }
>>
>> foo2.c: In function ‘foo’:
>> foo2.c:9:1: error: non-trivial conversion in ‘vector_cst’
>> 9 | }
>> | ^
>> svint32_t
>> int32x4_t
>> v2_4 = { 1, 2, 3, 4 };
>>
>> because simplify_permutation folds
>> VEC_PERM_EXPR< {1, 2, 3, 4}, {1, 2, 3, 4}, {0, 1, 2, 3, ...} >
>> into:
>> vector_cst {1, 2, 3, 4}
>>
>> and it complains during verify_gimple_assign_single because we don't
>> support assignment of vector_cst to VLA vector.
>>
>> I guess the issue really is that currently, only VEC_PERM_EXPR
>> supports lhs and rhs
>> to have vector types with differing lengths, and simplifying it to
>> other tree codes, like above,
>> will result in type errors ?
>
> That might be the case - Richard should know.
I don't see anything particularly special about VEC_PERM_EXPR here,
or about the VLA vs. VLS thing. We would have the same issue trying
to build a 128-bit vector from 2 64-bit vectors. And there are other
tree codes whose input types are/can be different from their output
types.
So it just seems like a normal type correctness issue: a VEC_PERM_EXPR
of type T needs to be replaced by something of type T. Whether T has a
constant size or a variable size doesn't matter.
> If so your type check
> is still too late, you should instead recognize that we are permuting
> a VLA vector and then refuse to go any of the non-VEC_PERM generating
> paths - that probably means just allowing the code == VEC_PERM_EXPR
> case and not any of the CTOR/CST/VIEW_CONVERT_EXPR cases?
Yeah. If we're talking about the match.pd code, I think only:
(if (sel.series_p (0, 1, 0, 1))
{ op0; }
(if (sel.series_p (0, 1, nelts, 1))
{ op1; }
need a type compatibility check. For fold_vec_perm I think
we should just rearrange:
gcc_assert (known_eq (TYPE_VECTOR_SUBPARTS (type), nelts)
&& known_eq (TYPE_VECTOR_SUBPARTS (TREE_TYPE (arg0)), nelts)
&& known_eq (TYPE_VECTOR_SUBPARTS (TREE_TYPE (arg1)), nelts));
if (TREE_TYPE (TREE_TYPE (arg0)) != TREE_TYPE (type)
|| TREE_TYPE (TREE_TYPE (arg1)) != TREE_TYPE (type))
return NULL_TREE;
so that the assert comes after the early-out.
It would be good at some point to relax fold_vec_perm to cases where the
outputs are a different length from the inputs, since the all-constant
VEC_PERM_EXPR above could be folded to a VECTOR_CST.
Thanks,
Richard
next prev parent reply other threads:[~2022-07-14 11:52 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-12 19:11 Prathamesh Kulkarni
2022-07-13 6:51 ` Richard Biener
2022-07-14 7:54 ` Prathamesh Kulkarni
2022-07-14 8:33 ` Richard Biener
2022-07-14 11:52 ` Richard Sandiford [this message]
2022-07-15 13:48 ` Prathamesh Kulkarni
2022-07-18 6:27 ` Richard Biener
2022-07-20 15:35 ` Prathamesh Kulkarni
2022-07-21 6:51 ` Richard Biener
2022-08-01 3:16 ` Prathamesh Kulkarni
2022-08-08 8:56 ` Richard Biener
2022-08-09 10:09 ` Prathamesh Kulkarni
2022-08-09 13:12 ` Richard Biener
2022-08-11 13:23 ` Prathamesh Kulkarni
2022-08-16 16:30 ` Richard Sandiford
2022-08-17 11:31 ` Richard Biener
2022-08-18 12:44 ` Prathamesh Kulkarni
2022-08-18 12:50 ` Prathamesh Kulkarni
2022-08-29 6:23 ` Prathamesh Kulkarni
2022-09-05 8:54 ` Prathamesh Kulkarni
2022-09-05 9:09 ` Richard Biener
2022-09-05 9:26 ` Prathamesh Kulkarni
2022-09-05 11:03 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=mpt5yjzx661.fsf@arm.com \
--to=richard.sandiford@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=prathamesh.kulkarni@linaro.org \
--cc=richard.guenther@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).