[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570 Jakub Jelinek changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #14 from Jakub Jelinek --- Fixed.
[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570 --- Comment #13 from Jakub Jelinek --- Author: jakub Date: Tue Mar 5 15:05:07 2019 New Revision: 269391 URL: https://gcc.gnu.org/viewcvs?rev=269391=gcc=rev Log: PR tree-optimization/89570 * match.pd (vec_cond into cond_op simplification): Don't use get_conditional_internal_fn, use as_internal_fn (cond_op). Modified: trunk/gcc/ChangeLog trunk/gcc/match.pd
[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570
--- Comment #12 from Jakub Jelinek ---
(In reply to rsand...@gcc.gnu.org from comment #6)
> (2) not splitting out the condition in a (vec_)cond_expr if it
> isn't supported as a stand-alone operation
(2) can be just documented as a requirement, unless we have targets that would
need further changes. So, if IFN_COND_* is supported for certain modes, then
vec_cmp{,u} needs to be supported for that mode too. I bet masked operations
are done using those bitmasks that are set by vec_cmp{,u}, dunno how exactly
for aarch64, but if x86 is adjusted to use those, it would be the mask
registers and
all the arithmetic etc. vector instructions masked.
[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570 --- Comment #11 from rguenther at suse dot de --- On Tue, 5 Mar 2019, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570 > > --- Comment #10 from Jakub Jelinek --- > That wouldn't help at all, the propagation in this PR is in forwprop4, after > vector lowering. The only way would be to make expanders have fallback code. But the reason for only allowing target supported vector GIMPLE is to avoid pessimizing code generation - in the case of compares/conds the chance is we end up with duplicated work.
[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570 --- Comment #10 from Jakub Jelinek --- That wouldn't help at all, the propagation in this PR is in forwprop4, after vector lowering.
[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570
--- Comment #9 from rsandifo at gcc dot gnu.org
---
(In reply to Jakub Jelinek from comment #8)
> For VEC_COND_EXPR, yes, it is pretty usual on many of the targets that you
> can only do:
> _1 = VEC_COND_EXPR <_2 == _3, {-1, -1, ...}, {0, 0, ...}>;
> (that is vcond/vcondu/vcondeq optab), and not:
> _4 = _2 == _3;
> (that is vec_cmp/vec_cmpu/vec_cmpeq optab).
> Quick grep for '"vcond' config/*/*.md shows that the former is likely in
> aarch64, arm, gcn, i386, ia64, mips, rs6000, s390, sparc, spu
> and the latter only in
> aarch64, gcn, i386, s390
> (haven't checked exact modes).
Could we get tree-vect-generic.c to "lower" stand-alone comparisons
into VEC_COND_EXPRs if they're not supported otherwise? I guess this
is all bound up with that discussion about having 4-op comparisons vs.
always having separate comparisons though.
[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570
--- Comment #8 from Jakub Jelinek ---
(In reply to rsand...@gcc.gnu.org from comment #6)
> I think there are two things here:
>
> (1) checking earlier for whether an ifn is supported.
>
> I think we should get genmatch to do that itself rather than
> manually do it for each expansion.
>
> (2) not splitting out the condition in a (vec_)cond_expr if it
> isn't supported as a stand-alone operation
>
> It looks like (2) is the real fix and (1) is a compile-time
> improvement that happens to make (2) latent in this case.
>
> How is it possible for a condition to be supported only in
> a VEC_COND_EXPR? Isn't a stand-alone condition equivalent
> to a VEC_COND_EXPR between {-1, ...} and {0, ...}?
Oops, I've committed already before reading your comments.
Yes, the committed patch does (1). Not really sure it needs to be done in
other patterns that don't introduce IFN_COND_*, those other take IFN_COND_* and
transform that to some other IFN_COND_*.
For VEC_COND_EXPR, yes, it is pretty usual on many of the targets that you can
only do:
_1 = VEC_COND_EXPR <_2 == _3, {-1, -1, ...}, {0, 0, ...}>;
(that is vcond/vcondu/vcondeq optab), and not:
_4 = _2 == _3;
(that is vec_cmp/vec_cmpu/vec_cmpeq optab).
Quick grep for '"vcond' config/*/*.md shows that the former is likely in
aarch64, arm, gcn, i386, ia64, mips, rs6000, s390, sparc, spu
and the latter only in
aarch64, gcn, i386, s390
(haven't checked exact modes).
[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570 --- Comment #7 from Jakub Jelinek --- Author: jakub Date: Tue Mar 5 08:44:21 2019 New Revision: 269385 URL: https://gcc.gnu.org/viewcvs?rev=269385=gcc=rev Log: PR tree-optimization/89570 * match.pd (vec_cond into cond_op simplification): Guard with vectorized_internal_fn_supported_p test and #if GIMPLE. * gcc.dg/pr89570.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr89570.c Modified: trunk/gcc/ChangeLog trunk/gcc/match.pd trunk/gcc/testsuite/ChangeLog
[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570
--- Comment #6 from rsandifo at gcc dot gnu.org
---
I think there are two things here:
(1) checking earlier for whether an ifn is supported.
I think we should get genmatch to do that itself rather than
manually do it for each expansion.
(2) not splitting out the condition in a (vec_)cond_expr if it
isn't supported as a stand-alone operation
It looks like (2) is the real fix and (1) is a compile-time
improvement that happens to make (2) latent in this case.
How is it possible for a condition to be supported only in
a VEC_COND_EXPR? Isn't a stand-alone condition equivalent
to a VEC_COND_EXPR between {-1, ...} and {0, ...}?
[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570 --- Comment #5 from rguenther at suse dot de --- On Mon, 4 Mar 2019, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570 > > Jakub Jelinek changed: > >What|Removed |Added > > Status|NEW |ASSIGNED >Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot > gnu.org > > --- Comment #4 from Jakub Jelinek --- > Created attachment 45880 > --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45880=edit > gcc9-pr89570.patch > > Untested fix. No way to test on aarch64 with SVE though (nor experience with > that in cross-testing). LGTM
[Bug tree-optimization/89570] [9 Regression] ICE in prepare_cmp_insn, at optabs.c:4001
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89570 Jakub Jelinek changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #4 from Jakub Jelinek --- Created attachment 45880 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=45880=edit gcc9-pr89570.patch Untested fix. No way to test on aarch64 with SVE though (nor experience with that in cross-testing).
