Re: [PATCH 1/2] Add vect_finish_stmt_generation for sequences
Richard Biener <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 17 Aug 2026, Richard Biener wrote: > On Mon, 17 Aug 2026, H.J. Lu wrote: > > > On Sun, Aug 16, 2026 at 2:30 PM Andrea Pinski > > <[email protected]> wrote: > > > > > > On Sat, Aug 15, 2026 at 11:01 PM H.J. Lu <[email protected]> wrote: > > > > > > > > On Sun, Aug 16, 2026 at 6:36 AM H.J. Lu <[email protected]> wrote: > > > > > > > > > > On Fri, Aug 14, 2026 at 8:13 PM Richard Biener <[email protected]> wrote: > > > > > > > > > > > > The following attempts to make using gimple_build easier during > > > > > > vectorizer code generation by providing an overload of > > > > > > vect_finish_stmt_generation that handles a built sequence. > > > > > > In addition to inserting and finishing stmts on the sequence the > > > > > > ultimate result is made to be based on the passed vectorizer > > > > > > temporary (that gets us the fancy names). > > > > > > > > > > > > I have converted one instance in vectorizable_conversion which > > > > > > I'll need to handle both NOP_EXPR and VIEW_CONVERT_EXPR which > > > > > > gimple_build happily does correctly, but vect_gimple_build does > > > > > > not. > > > > > > > > > > > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > > > > > > > > > > > Does this look sensible? I can of course sepecial-case > > > > > > VIEW_CONVERT_EXPR manually as well. > > > > > > > > > > > > * tree-vect-stmts.cc (vect_finish_stmt_generation): New > > > > > > overload for gimple_build sequences. > > > > > > (vectorizable_conversion): Convert one instance of > > > > > > vect_gimple_build to gimple_build plus > > > > > > vect_finish_stmt_generation. > > > > > > --- > > > > > > gcc/tree-vect-stmts.cc | 43 ++++++++++++++++++++++++++++++++---------- > > > > > > 1 file changed, 33 insertions(+), 10 deletions(-) > > > > > > > > > > > > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc > > > > > > index a023977bc51..44090015e7a 100644 > > > > > > --- a/gcc/tree-vect-stmts.cc > > > > > > +++ b/gcc/tree-vect-stmts.cc > > > > > > @@ -1260,7 +1260,7 @@ vect_get_vec_defs (vec_info *, slp_tree slp_node, > > > > > > > > > > > > /* Helper function called by vect_finish_replace_stmt and > > > > > > vect_finish_stmt_generation. Set the location of the new > > > > > > - statement and create and return a stmt_vec_info for it. */ > > > > > > + statement. */ > > > > > > > > > > > > static void > > > > > > vect_finish_stmt_generation_1 (vec_info *, > > > > > > @@ -1285,8 +1285,7 @@ vect_finish_stmt_generation_1 (vec_info *, > > > > > > } > > > > > > > > > > > > /* Replace the scalar statement STMT_INFO with a new vector statement VEC_STMT, > > > > > > - which sets the same scalar result as STMT_INFO did. Create and return a > > > > > > - stmt_vec_info for VEC_STMT. */ > > > > > > + which sets the same scalar result as STMT_INFO did. */ > > > > > > > > > > > > void > > > > > > vect_finish_replace_stmt (vec_info *vinfo, > > > > > > @@ -1302,7 +1301,7 @@ vect_finish_replace_stmt (vec_info *vinfo, > > > > > > } > > > > > > > > > > > > /* Add VEC_STMT to the vectorized implementation of STMT_INFO and insert it > > > > > > - before *GSI. Create and return a stmt_vec_info for VEC_STMT. */ > > > > > > + before *GSI. */ > > > > > > > > > > > > void > > > > > > vect_finish_stmt_generation (vec_info *vinfo, > > > > > > @@ -1344,6 +1343,31 @@ vect_finish_stmt_generation (vec_info *vinfo, > > > > > > vect_finish_stmt_generation_1 (vinfo, stmt_info, vec_stmt); > > > > > > } > > > > > > > > > > > > +/* Add the stmts in STMTS to the vectorized implementation of STMT_INFO and > > > > > > + insert them before *GSI. Make sure the final stmt has a result > > > > > > + based on VAR. */ > > > > > > + > > > > > > +void > > > > > > +vect_finish_stmt_generation (vec_info *vinfo, > > > > > > + stmt_vec_info stmt_info, gimple_seq &stmts, > > > > > > + tree var, gimple_stmt_iterator *gsi) > > > > > > +{ > > > > > > + auto si = gsi_start (stmts); > > > > > > + while (!gsi_end_p (si)) > > > > > > + { > > > > > > + gimple *new_stmt = gsi_stmt (si); > > > > > > + gsi_remove (&si, false); > > > > > > + if (gsi_end_p (si)) > > > > > > + { > > > > > > + tree lhs = gimple_get_lhs (new_stmt); > > > > > > + gcc_assert (!SSA_NAME_VAR (lhs)); > > > > > > + SET_SSA_NAME_VAR_OR_IDENTIFIER (lhs, var); > > > > > > + } > > > > > > + vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi); > > > > > > + } > > > > > > +} > > > > > > + > > > > > > + > > > > > > /* We want to vectorize a call to combined function CFN with function > > > > > > decl FNDECL, using VECTYPE_OUT as the type of the output and VECTYPE_IN > > > > > > as the types of all inputs. Check whether this is possible using > > > > > > @@ -5797,12 +5821,11 @@ vectorizable_conversion (vec_info *vinfo, > > > > > > } > > > > > > else > > > > > > { > > > > > > - new_stmt = vect_gimple_build (vec_dest, code1, vop0); > > > > > > - new_temp = make_ssa_name (vec_dest, new_stmt); > > > > > > - gimple_set_lhs (new_stmt, new_temp); > > > > > > - vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi); > > > > > > - > > > > > > - slp_node->push_vec_def (new_stmt); > > > > > > + gimple_seq stmts = NULL; > > > > > > + new_temp = gimple_build (&stmts, code1, vectype_out, vop0); > > > > > > + vect_finish_stmt_generation (vinfo, stmt_info, > > > > > > + stmts, vec_dest, gsi); > > > > > > + slp_node->push_vec_def (new_temp); > > > > > > } > > > > > > } > > > > > > break; > > > > > > -- > > > > > > 2.51.0 > > > > > > > > > > > > > > > > I need the patch enclosed here to avoid ICE in 2 testcases included > > > > > > > > Here is the updated patch with 3 tests. > > > > > > > > > in the patch. But I got another ICE in bad.c: > > > > > > > > > > [hjl@gnu-zen4-1 gcc]$ ./xgcc -B./ -S -O2 -march=x86-64 ~/bugs/gcc/cvise-1/bad.c > > > > > during GIMPLE pass: thread > > > > > /export/home/hjl/bugs/gcc/cvise-1/bad.c: In function ‘collate_output___o_2’: > > > > > /export/home/hjl/bugs/gcc/cvise-1/bad.c:20:1: internal compiler error: > > > > > Segmentation fault > > > > > > > > This is > > > > > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126876 > > > > > > The big comment about HJL's patch is `code1 == NOP_EXPR` should be > > > `CONVERT_EXPR_CODE_P (code1)` instead. > > > > > > > CONVERT_EXPR_CODE_P works for me. > > Your testcases do not ICE for me. Btw, your patch essentially > implements the "classical" way as if I had added a > else if (code1 == VIEW_CONVERT_EXPR) case or alternatively > rejected noop conversions from vectorizable_conversion, leaving > them to vectorizable_assignment where they were handled before. > > So I'm going to skip all these issues, re-testing the folding patch > alone as below. Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed. Richard. > Richard. > > From 4eb4f720370cd02d5b5ef0ed3add4b9eb57d021a Mon Sep 17 00:00:00 2001 > From: Richard Biener <[email protected]> > Date: Thu, 13 Aug 2026 14:57:56 +0200 > Subject: [PATCH] middle-end/126788 - more vector folding > To: [email protected] > > The last match.pd update for double-conversion folding didn't handle > the case where a vector conversion would go away because > supportable_convert_operation doesn't consider a noop conversion > (or a sign conversion). The following rectifies this, allowing > a NOP_EXPR and VIEW_CONVERT_EXPR for same mode types as supportable > conversion as we can RTL expand that just fine. > > This shows that the vectorizer routed sign-conversions through > vectorizable_assignment but now vectorizable_conversion would > handle it, emitting NOP_EXPRs (which is fine) instead of > VIEW_CONVERT_EXPRs (which we declared canonical for vectors). > This would confuse some foldings, leading to testsuite FAILs, > so make vectorizable_conversion also prefer VIEW_CONVERT_EXPRs > here. > > PR middle-end/126788 > * optabs-tree.cc (supportable_convert_operation): For same > modes allow NOP_EXPR and VIEW_CONVERT_EXPR. > * tree-ssa-forwprop.cc (simplify_vector_constructor): Avoid > converting the vector to be used for blending into the > result to the type of the permutation. > * tree-vect-stmts.cc (vectorizable_conversion): Leave > nop-conversions to vectorizable_assignment. > > * gcc.target/i386/pr126788.c: New testcase. > --- > gcc/optabs-tree.cc | 4 ++++ > gcc/testsuite/gcc.target/i386/pr126788.c | 24 ++++++++++++++++++++++++ > gcc/tree-ssa-forwprop.cc | 1 + > gcc/tree-vect-stmts.cc | 9 ++++++++- > 4 files changed, 37 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr126788.c > > diff --git a/gcc/optabs-tree.cc b/gcc/optabs-tree.cc > index 5fbdcd3a730..5360a509f8d 100644 > --- a/gcc/optabs-tree.cc > +++ b/gcc/optabs-tree.cc > @@ -375,6 +375,10 @@ supportable_convert_operation (enum tree_code code, > if (!VECTOR_MODE_P (m1) || !VECTOR_MODE_P (m2)) > return false; > > + if (m1 == m2 > + && (CONVERT_EXPR_CODE_P (code) || code == VIEW_CONVERT_EXPR)) > + return true; > + > /* First check if we can done conversion directly. */ > if ((code == FIX_TRUNC_EXPR > && can_fix_p (m1,m2,TYPE_UNSIGNED (vectype_out), &truncp) > diff --git a/gcc/testsuite/gcc.target/i386/pr126788.c b/gcc/testsuite/gcc.target/i386/pr126788.c > new file mode 100644 > index 00000000000..0448284e8f6 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr126788.c > @@ -0,0 +1,24 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O -msse2" } */ > + > +typedef int v2si __attribute__((vector_size (8))); > +typedef unsigned int v2usi __attribute__((vector_size (8))); > +typedef long long v2di __attribute__((vector_size (16))); > + > +v2si > +f1 (v2si a, v2si b) > +{ > + > + v2di z = __builtin_convertvector (a, v2di); > + return __builtin_convertvector (z, v2si); > +} > + > +v2usi > +f2 (v2si a, v2si b) > +{ > + > + v2di z = __builtin_convertvector (a, v2di); > + return __builtin_convertvector (z, v2usi); > +} > + > +/* { dg-final { scan-assembler-not "xmm" { target { ! ia32 } } } } */ > diff --git a/gcc/tree-ssa-forwprop.cc b/gcc/tree-ssa-forwprop.cc > index 0531311444b..8cc017aa620 100644 > --- a/gcc/tree-ssa-forwprop.cc > +++ b/gcc/tree-ssa-forwprop.cc > @@ -4381,6 +4381,7 @@ simplify_vector_constructor (gimple_stmt_iterator *gsi) > /* For a real orig[1] (no splat, constant etc.) we might need to > nop-convert it. Do so here. */ > if (orig[1] && orig[1] != error_mark_node > + && !converted_orig1 > && !useless_type_conversion_p (perm_type, TREE_TYPE (orig[1])) > && tree_nop_conversion_p (TREE_TYPE (perm_type), > TREE_TYPE (TREE_TYPE (orig[1])))) > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc > index 98533277034..344aeddfb5a 100644 > --- a/gcc/tree-vect-stmts.cc > +++ b/gcc/tree-vect-stmts.cc > @@ -5500,7 +5500,14 @@ vectorizable_conversion (vec_info *vinfo, > { > gcc_assert (converts.length () <= 2); > if (converts.length () == 1) > - code1 = converts[0].second; > + { > + code1 = converts[0].second; > + if (CONVERT_EXPR_CODE_P (code) > + && tree_nop_conversion_p (TREE_TYPE (vectype_out), > + TREE_TYPE (vectype_in))) > + /* NOP conversions are handled by vectorizable_assignment. */ > + return false; > + } > else > { > cvt_type = NULL_TREE; > -- Richard Biener <[email protected]> SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)