Re: [PATCH 1/2] Add vect_finish_stmt_generation for sequences

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.

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;
-- 
2.51.0