[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).