* [PATCH] tree-vect-generic: Fix up ICE with SSA_NAME_OCCURS_IN_ABNORMAL_PHI [PR109392] @ 2023-04-05 8:58 Jakub Jelinek 2023-04-05 12:11 ` Richard Biener 0 siblings, 1 reply; 4+ messages in thread From: Jakub Jelinek @ 2023-04-05 8:58 UTC (permalink / raw) To: Richard Biener, Jeff Law; +Cc: gcc-patches Hi! tree_vect_extract uses gimple-fold/match.pd to attempt to simplify the BIT_FIELD_REF immediately. Unfortunately, maybe_push_res_to_seq has: /* Play safe and do not allow abnormals to be mentioned in newly created statements. */ for (unsigned int i = 0; i < num_ops; ++i) if (TREE_CODE (ops[i]) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[i])) return NULL_TREE; if (num_ops > 0 && COMPARISON_CLASS_P (ops[0])) for (unsigned int i = 0; i < 2; ++i) if (TREE_CODE (TREE_OPERAND (ops[0], i)) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (TREE_OPERAND (ops[0], i))) return NULL_TREE; and so can fail in presence of SSA_NAME_OCCURS_IN_ABNORMAL_PHI SSA_NAMEs, where we then trigger the assert that maybe_push_res_to_seq doesn't return NULL. The above is perfectly reasonable when trying to actually simplify something that has been already created and let us just punt, but in the tree_vect_extract case we have to build something always. The following patch fixes it by building the BIT_FIELD_REF manually in that case if the build+simplification failed. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2023-04-05 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/109392 * tree-vect-generic.cc (tree_vec_extract): If maybe_push_res_to_seq fails, build BIT_FIELD_REF insn without trying to simplify it. * gcc.dg/pr109392.c: New test. --- gcc/tree-vect-generic.cc.jj 2023-03-23 10:02:18.997935620 +0100 +++ gcc/tree-vect-generic.cc 2023-04-04 14:28:32.977729134 +0200 @@ -174,7 +174,16 @@ tree_vec_extract (gimple_stmt_iterator * opr.resimplify (NULL, follow_all_ssa_edges); gimple_seq stmts = NULL; tree res = maybe_push_res_to_seq (&opr, &stmts); - gcc_assert (res); + if (!res) + { + /* This can happen if SSA_NAME_OCCURS_IN_ABNORMAL_PHI are + used. Build BIT_FIELD_REF manually otherwise. */ + t = build3 (BIT_FIELD_REF, type, t, bitsize, bitpos); + res = make_ssa_name (type); + gimple *g = gimple_build_assign (res, t); + gsi_insert_before (gsi, g, GSI_SAME_STMT); + return res; + } gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT); return res; } --- gcc/testsuite/gcc.dg/pr109392.c.jj 2023-04-04 14:36:03.096109943 +0200 +++ gcc/testsuite/gcc.dg/pr109392.c 2023-04-04 14:35:39.784452751 +0200 @@ -0,0 +1,15 @@ +/* PR tree-optimization/109392 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wno-psabi" } */ + +typedef short __attribute__ ((__vector_size__ (64))) V; +V v, w; +void bar (void) __attribute__((returns_twice)); + +V +foo (V a, V b) +{ + bar (); + b &= v < b; + return (V) { foo (b, w)[3], (V) {}[3] }; +} Jakub ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tree-vect-generic: Fix up ICE with SSA_NAME_OCCURS_IN_ABNORMAL_PHI [PR109392] 2023-04-05 8:58 [PATCH] tree-vect-generic: Fix up ICE with SSA_NAME_OCCURS_IN_ABNORMAL_PHI [PR109392] Jakub Jelinek @ 2023-04-05 12:11 ` Richard Biener 2023-04-11 8:04 ` Jakub Jelinek 0 siblings, 1 reply; 4+ messages in thread From: Richard Biener @ 2023-04-05 12:11 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Jeff Law, gcc-patches > Am 05.04.2023 um 10:58 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>: > > Hi! > > tree_vect_extract uses gimple-fold/match.pd to attempt to simplify > the BIT_FIELD_REF immediately. > Unfortunately, maybe_push_res_to_seq has: > /* Play safe and do not allow abnormals to be mentioned in > newly created statements. */ > for (unsigned int i = 0; i < num_ops; ++i) > if (TREE_CODE (ops[i]) == SSA_NAME > && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[i])) > return NULL_TREE; > > if (num_ops > 0 && COMPARISON_CLASS_P (ops[0])) > for (unsigned int i = 0; i < 2; ++i) > if (TREE_CODE (TREE_OPERAND (ops[0], i)) == SSA_NAME > && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (TREE_OPERAND (ops[0], i))) > return NULL_TREE; > and so can fail in presence of SSA_NAME_OCCURS_IN_ABNORMAL_PHI SSA_NAMEs, > where we then trigger the assert that maybe_push_res_to_seq doesn't return > NULL. The above is perfectly reasonable when trying to actually simplify > something that has been already created and let us just punt, but in the > tree_vect_extract case we have to build something always. > > The following patch fixes it by building the BIT_FIELD_REF manually in > that case if the build+simplification failed. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok, but maybe there’s a gimple_build overload that meanwhile implements the desired semantics? It would probably need to specify the valueization hook to follow SSA edges beyond the sequence we’re currently building. Richard > 2023-04-05 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/109392 > * tree-vect-generic.cc (tree_vec_extract): If maybe_push_res_to_seq > fails, build BIT_FIELD_REF insn without trying to simplify it. > > * gcc.dg/pr109392.c: New test. > > --- gcc/tree-vect-generic.cc.jj 2023-03-23 10:02:18.997935620 +0100 > +++ gcc/tree-vect-generic.cc 2023-04-04 14:28:32.977729134 +0200 > @@ -174,7 +174,16 @@ tree_vec_extract (gimple_stmt_iterator * > opr.resimplify (NULL, follow_all_ssa_edges); > gimple_seq stmts = NULL; > tree res = maybe_push_res_to_seq (&opr, &stmts); > - gcc_assert (res); > + if (!res) > + { > + /* This can happen if SSA_NAME_OCCURS_IN_ABNORMAL_PHI are > + used. Build BIT_FIELD_REF manually otherwise. */ > + t = build3 (BIT_FIELD_REF, type, t, bitsize, bitpos); > + res = make_ssa_name (type); > + gimple *g = gimple_build_assign (res, t); > + gsi_insert_before (gsi, g, GSI_SAME_STMT); > + return res; > + } > gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT); > return res; > } > --- gcc/testsuite/gcc.dg/pr109392.c.jj 2023-04-04 14:36:03.096109943 +0200 > +++ gcc/testsuite/gcc.dg/pr109392.c 2023-04-04 14:35:39.784452751 +0200 > @@ -0,0 +1,15 @@ > +/* PR tree-optimization/109392 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -Wno-psabi" } */ > + > +typedef short __attribute__ ((__vector_size__ (64))) V; > +V v, w; > +void bar (void) __attribute__((returns_twice)); > + > +V > +foo (V a, V b) > +{ > + bar (); > + b &= v < b; > + return (V) { foo (b, w)[3], (V) {}[3] }; > +} > > Jakub > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tree-vect-generic: Fix up ICE with SSA_NAME_OCCURS_IN_ABNORMAL_PHI [PR109392] 2023-04-05 12:11 ` Richard Biener @ 2023-04-11 8:04 ` Jakub Jelinek 2023-04-11 10:38 ` Richard Biener 0 siblings, 1 reply; 4+ messages in thread From: Jakub Jelinek @ 2023-04-11 8:04 UTC (permalink / raw) To: Richard Biener; +Cc: Jeff Law, gcc-patches On Wed, Apr 05, 2023 at 02:11:08PM +0200, Richard Biener wrote: > Ok, but maybe there’s a gimple_build overload that meanwhile implements > the desired semantics? It would probably need to specify the valueization > hook to follow SSA edges beyond the sequence we’re currently building. Jeff has apparently committed the patch in the meantime. For gimple_build, did you mean to use it just for this fallback case, or instead of the initial resimplification as well? > > 2023-04-05 Jakub Jelinek <jakub@redhat.com> > > > > PR tree-optimization/109392 > > * tree-vect-generic.cc (tree_vec_extract): If maybe_push_res_to_seq > > fails, build BIT_FIELD_REF insn without trying to simplify it. > > > > * gcc.dg/pr109392.c: New test. > > > > --- gcc/tree-vect-generic.cc.jj 2023-03-23 10:02:18.997935620 +0100 > > +++ gcc/tree-vect-generic.cc 2023-04-04 14:28:32.977729134 +0200 > > @@ -174,7 +174,16 @@ tree_vec_extract (gimple_stmt_iterator * > > opr.resimplify (NULL, follow_all_ssa_edges); > > gimple_seq stmts = NULL; > > tree res = maybe_push_res_to_seq (&opr, &stmts); > > - gcc_assert (res); > > + if (!res) > > + { > > + /* This can happen if SSA_NAME_OCCURS_IN_ABNORMAL_PHI are > > + used. Build BIT_FIELD_REF manually otherwise. */ > > + t = build3 (BIT_FIELD_REF, type, t, bitsize, bitpos); > > + res = make_ssa_name (type); > > + gimple *g = gimple_build_assign (res, t); > > + gsi_insert_before (gsi, g, GSI_SAME_STMT); > > + return res; > > + } > > gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT); > > return res; > > } Jakub ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tree-vect-generic: Fix up ICE with SSA_NAME_OCCURS_IN_ABNORMAL_PHI [PR109392] 2023-04-11 8:04 ` Jakub Jelinek @ 2023-04-11 10:38 ` Richard Biener 0 siblings, 0 replies; 4+ messages in thread From: Richard Biener @ 2023-04-11 10:38 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Jeff Law, gcc-patches On Tue, 11 Apr 2023, Jakub Jelinek wrote: > On Wed, Apr 05, 2023 at 02:11:08PM +0200, Richard Biener wrote: > > Ok, but maybe there?s a gimple_build overload that meanwhile implements > > the desired semantics? It would probably need to specify the valueization > > hook to follow SSA edges beyond the sequence we?re currently building. > > Jeff has apparently committed the patch in the meantime. > For gimple_build, did you mean to use it just for this fallback case, > or instead of the initial resimplification as well? I was originally thinking of the initial resimplification. But yes, using gimple_build would have simplified it to if (!res) res = gimple_build (&stmts, BIT_FIELD_REF, type, t, bitsize, bitpos); With the gsi overloads the whole function should now be able to be turned into return gimple_build (gsi, true, GSI_SAME_STMT, UNKNOWN_LOCATION, BIT_FIELD_REF, type, t, bitsize, bitpos); I think. Note that's not 100% equivalent to what we do now, so it's probably better to defer to stage1. What it doesn't do is /* We're using the resimplify API and maybe_push_res_to_seq to simplify the BIT_FIELD_REF but restrict the simplification to a single stmt while at the same time following SSA edges for simplification with already emitted CTORs. */ which is achieved by passing NULL as the gimple_seq * argument to .resimplify. The above replacement would instead do what a later forwprop pass would do when folding. ISTR I wanted to do minimal changes when rewriting this. Richard. > > > 2023-04-05 Jakub Jelinek <jakub@redhat.com> > > > > > > PR tree-optimization/109392 > > > * tree-vect-generic.cc (tree_vec_extract): If maybe_push_res_to_seq > > > fails, build BIT_FIELD_REF insn without trying to simplify it. > > > > > > * gcc.dg/pr109392.c: New test. > > > > > > --- gcc/tree-vect-generic.cc.jj 2023-03-23 10:02:18.997935620 +0100 > > > +++ gcc/tree-vect-generic.cc 2023-04-04 14:28:32.977729134 +0200 > > > @@ -174,7 +174,16 @@ tree_vec_extract (gimple_stmt_iterator * > > > opr.resimplify (NULL, follow_all_ssa_edges); > > > gimple_seq stmts = NULL; > > > tree res = maybe_push_res_to_seq (&opr, &stmts); > > > - gcc_assert (res); > > > + if (!res) > > > + { > > > + /* This can happen if SSA_NAME_OCCURS_IN_ABNORMAL_PHI are > > > + used. Build BIT_FIELD_REF manually otherwise. */ > > > + t = build3 (BIT_FIELD_REF, type, t, bitsize, bitpos); > > > + res = make_ssa_name (type); > > > + gimple *g = gimple_build_assign (res, t); > > > + gsi_insert_before (gsi, g, GSI_SAME_STMT); > > > + return res; > > > + } > > > gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT); > > > return res; > > > } > > Jakub > > -- Richard Biener <rguenther@suse.de> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg) ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-04-11 10:38 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-05 8:58 [PATCH] tree-vect-generic: Fix up ICE with SSA_NAME_OCCURS_IN_ABNORMAL_PHI [PR109392] Jakub Jelinek 2023-04-05 12:11 ` Richard Biener 2023-04-11 8:04 ` Jakub Jelinek 2023-04-11 10:38 ` Richard Biener
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).