Re: [PATCH 1/2] Add vect_finish_stmt_generation for sequences
"H.J. Lu" <[email protected]>
| Newsgroups | gmane.comp.gcc.patches |
|---|---|
| Message-ID | <CAMe9rOp02XThM+CmaW7+YGtwp07OeLOCxHUzkGz3qC+Q406xVw@mail.gmail.com> |
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 -- H.J.
0001-Fix-Add-vect_finish_stmt_generation-for-sequences.patch
(text/x-patch, 7.5 KB)
From 086c15a293ea571308280db3d16cd276ccb63391 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <[email protected]> Date: Sat, 15 Aug 2026 08:35:23 +0800 Subject: [PATCH] Fix Add vect_finish_stmt_generation for sequences Signed-off-by: H.J. Lu <[email protected]> --- gcc/testsuite/g++.target/i386/pr126788-1.C | 39 +++++ gcc/testsuite/g++.target/i386/pr126788-2.C | 169 +++++++++++++++++++++ gcc/testsuite/g++.target/i386/pr126788-3.C | 30 ++++ gcc/tree-vect-stmts.cc | 9 ++ 4 files changed, 247 insertions(+) create mode 100644 gcc/testsuite/g++.target/i386/pr126788-1.C create mode 100644 gcc/testsuite/g++.target/i386/pr126788-2.C create mode 100644 gcc/testsuite/g++.target/i386/pr126788-3.C diff --git a/gcc/testsuite/g++.target/i386/pr126788-1.C b/gcc/testsuite/g++.target/i386/pr126788-1.C new file mode 100644 index 00000000000..77e09f9ea54 --- /dev/null +++ b/gcc/testsuite/g++.target/i386/pr126788-1.C @@ -0,0 +1,39 @@ +/* { dg-do compile } */ +/* { dg-options "-O -msse2 -std=gnu++17" } */ +/* Keep labels and directives ('.cfi_startproc', '.cfi_endproc'). */ +/* { dg-final { check-function-bodies "**" "" "" { target { ! ia32 } } {^\t?\.} } } */ + +typedef int v2si __attribute__((vector_size (8))); +typedef unsigned int v2usi __attribute__((vector_size (8))); +typedef long long v2di __attribute__((vector_size (16))); + +/* +**_Z5func1Dv2_iS_: +**.LFB[0-9]+: +** .cfi_startproc +** ret +** .cfi_endproc +**... +*/ + +v2si +func1 (v2si a, v2si b) +{ + v2di z = __builtin_convertvector (a, v2di); + return __builtin_convertvector (z, v2si); +} + +/* +**_Z5func2Dv2_iS_: +**.LFB[0-9]+: +** .cfi_startproc +** ret +** .cfi_endproc +**... +*/ +v2usi +func2 (v2si a, v2si b) +{ + v2di z = __builtin_convertvector (a, v2di); + return __builtin_convertvector (z, v2usi); +} diff --git a/gcc/testsuite/g++.target/i386/pr126788-2.C b/gcc/testsuite/g++.target/i386/pr126788-2.C new file mode 100644 index 00000000000..2cd2ca2d76a --- /dev/null +++ b/gcc/testsuite/g++.target/i386/pr126788-2.C @@ -0,0 +1,169 @@ +/* { dg-do compile } */ +/* { dg-options "-fno-checking -O2 -march=x86-64-v4 -std=c++20" } */ + +char __parse_integer___buf[32]; +template <typename _Tp> using remove_reference_t = _Tp; +template <typename _Tp, typename _Up> +constexpr bool is_same_v = __is_same (_Tp, _Up); +template <typename _Tp, typename _Up> +concept __same_as = is_same_v<_Tp, _Up>; +template <typename _Tp, typename _Up> +concept same_as = __same_as<_Up, _Tp>; +struct __is_integer +{ + enum + { + __value = 1 + }; +}; +using wstring_view = int; +template <typename, typename> class basic_format_context; +template <typename> struct basic_format_parse_context +{ + int *begin (); + int *end (); +}; +template <typename, typename> struct formatter; +namespace __format +{ +template <typename _CharT> +concept __char = same_as<_CharT, wchar_t>; +template <typename> struct _Sink_iter +{ +}; +template <typename _CharT> +using __format_context = basic_format_context<_CharT, _CharT>; +} // namespace __format +using wformat_context = __format::__format_context<wchar_t>; +namespace __format +{ +template <typename _CharT> +void +__parse_integer (_CharT *__first, _CharT *) +{ + for (int __i = 0; __i < 32 && __first + __i; ++__i) + __parse_integer___buf[__i] = __i; + __parse_integer (__parse_integer___buf, __parse_integer___buf); +} +enum _Pres_type +{ +}; +template <typename _CharT> struct _Spec +{ + void + _M_parse_width (int *__first, int *__last, + basic_format_parse_context<_CharT>) + { + __parse_integer (__first, __last); + } +}; +template <__char _CharT> struct __formatter_int +{ + _Pres_type _AsInteger; + _CharT + _M_do_parse (basic_format_parse_context<_CharT> __pc, _Pres_type) + { + _Spec<_CharT> __spec; + auto __last = __pc.end (), __first = __pc.begin (); + __spec._M_parse_width (__first, __last, __pc); + return 0; + } + template <typename> + _CharT + _M_parse (basic_format_parse_context<_CharT> __pc) + { + _M_do_parse (__pc, _AsInteger); + return 0; + } +}; +template <typename> +constexpr bool __is_formattable_integer = __is_integer::__value; +template <typename _Tp> +concept __formattable_integer = __is_formattable_integer<_Tp>; +} // namespace __format +template <__format::__formattable_integer _Tp, __format::__char _CharT> +struct formatter<_Tp, _CharT> +{ + _CharT + parse (basic_format_parse_context<_CharT> __pc) + { + _M_f.template _M_parse<_Tp> (__pc); + return 0; + } + __format::__formatter_int<_CharT> _M_f; +}; +namespace __format +{ +enum _Arg_t +{ + _Arg_u128 +}; +} +int _M_val; +__format::_Arg_t _M_type; +struct +{ + template <typename _Visitor> + void + _M_visit (_Visitor) + { + switch (_M_type) + { + using enum __format::_Arg_t; + case _Arg_u128: + _Visitor () (_M_val); + } + } +} __visit_format_arg___arg; +template <typename _Visitor> +void +__visit_format_arg (_Visitor) +{ + __visit_format_arg___arg._M_visit (_Visitor ()); +} +template <typename, typename> struct basic_format_context +{ + template <typename _Tp> using formatter_type = formatter<_Tp, wchar_t>; +}; +namespace __format +{ +struct : basic_format_parse_context<wchar_t> +{ +} _M_pc; +void _M_scan (); +struct _Scanner +{ + _Scanner (int); + virtual void _M_format_arg (unsigned long); +}; +template <typename _Out, typename _CharT> struct _Formatting_scanner : _Scanner +{ + _Formatting_scanner (basic_format_context<_Out, _CharT>, int __str) + : _Scanner (__str) + { + } + void + _M_format_arg (unsigned long) + { + __visit_format_arg ( + [] (auto __arg) + { + typename basic_format_context<_Out, _CharT>::formatter_type< + remove_reference_t<decltype (__arg)>> + __f; + __f.parse (_M_pc); + }); + } +}; +template <unsigned = 0> +_Sink_iter<wchar_t> +__do_vformat_to (_Sink_iter<wchar_t> x, int __fmt, + __format_context<wchar_t> &__ctx) +{ + _Formatting_scanner __scanner (__ctx, __fmt); + _M_scan (); + return x; +} +template _Sink_iter<wchar_t> __do_vformat_to (_Sink_iter<wchar_t>, + wstring_view, wformat_context &); +} diff --git a/gcc/testsuite/g++.target/i386/pr126788-3.C b/gcc/testsuite/g++.target/i386/pr126788-3.C new file mode 100644 index 00000000000..17723cdcba5 --- /dev/null +++ b/gcc/testsuite/g++.target/i386/pr126788-3.C @@ -0,0 +1,30 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -march=x86-64-v4 -std=c++17" } */ + +extern int _M_current; +extern int end (); +struct color +{ + char r; + struct + { + unsigned char g; + unsigned char b; + } m_24bit; + color (char g, char b) + { + m_24bit.g = g; + m_24bit.b = b; + } +}; +void +set_style_bg_color (color) +{ + for (;;) + { + const unsigned char g = _M_current; + if (end ()) + break; + set_style_bg_color (color (g, _M_current)); + } +} diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc index 070981dbdb3..00a2d54826e 100644 --- a/gcc/tree-vect-stmts.cc +++ b/gcc/tree-vect-stmts.cc @@ -5826,6 +5826,15 @@ vectorizable_conversion (vec_info *vinfo, vect_finish_stmt_generation (vinfo, stmt_info, new_stmt, gsi); slp_node->push_vec_def (new_temp2); } + else if (code1 == NOP_EXPR) + { + 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); + } else { gimple_seq stmts = NULL; -- 2.55.0