Message ID | 31c05be7-64bf-8d93-934c-63262e082e68@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | vect: Try folding first for shifted value generation [PR107240] | expand |
On Wed, Oct 19, 2022 at 5:18 AM Kewen.Lin <linkw@linux.ibm.com> wrote: > > Hi, > > As PR107240 shows, when both the value to be shifted and the > count used for shifting are constants, it doesn't actually > requires a target to support vector shift operations. > > This patch is to try fold_build2 for the generation of the > shifted value first, if it's folded, the shift is gone, > otherwise it's the same as before. > > It can help to make the failures of vect-bitfield-write-{2,3}.c > gone on Power. > > Bootstrapped and regtested on x86_64-redhat-linux, > aarch64-linux-gnu and powerpc64{,le}-linux-gnu. > > Is it ok for trunk? > > BR, > Kewen > ----- > PR tree-optimization/107240 > > gcc/ChangeLog: > > * tree-vect-patterns.cc (vect_recog_bit_insert_pattern): Attempt to > fold shifted value. > --- > gcc/tree-vect-patterns.cc | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc > index 6afd57a50c4..3beda774ec3 100644 > --- a/gcc/tree-vect-patterns.cc > +++ b/gcc/tree-vect-patterns.cc > @@ -2098,9 +2098,11 @@ vect_recog_bit_insert_pattern (vec_info *vinfo, stmt_vec_info stmt_info, > tree shifted = value; > if (shift_n) > { > + tree shifted_tmp > + = fold_build2 (LSHIFT_EXPR, container_type, value, shift); > pattern_stmt > = gimple_build_assign (vect_recog_temp_ssa_var (container_type), > - LSHIFT_EXPR, value, shift); > + shifted_tmp); The canonical way would be to use gimple_seq stmts = NULL; shifted = gimple_build (&stmts, LSHIFT_EXPR, container_type, value, shift); if (!gimple_seq_empty_p (stmts)) append_pattern_def_seq (vinfo, stmt_info, gimple_seq_first_stmt (stmts)); That also avoids the spurious val = constant; with your patch. OK if that works. thanks, Richard. > append_pattern_def_seq (vinfo, stmt_info, pattern_stmt); > shifted = gimple_get_lhs (pattern_stmt); > } > -- > 2.27.0
Hi Richi, on 2022/10/19 15:43, Richard Biener wrote: > On Wed, Oct 19, 2022 at 5:18 AM Kewen.Lin <linkw@linux.ibm.com> wrote: >> >> Hi, >> >> As PR107240 shows, when both the value to be shifted and the >> count used for shifting are constants, it doesn't actually >> requires a target to support vector shift operations. >> >> This patch is to try fold_build2 for the generation of the >> shifted value first, if it's folded, the shift is gone, >> otherwise it's the same as before. >> >> It can help to make the failures of vect-bitfield-write-{2,3}.c >> gone on Power. >> >> Bootstrapped and regtested on x86_64-redhat-linux, >> aarch64-linux-gnu and powerpc64{,le}-linux-gnu. >> >> Is it ok for trunk? >> >> BR, >> Kewen >> ----- >> PR tree-optimization/107240 >> >> gcc/ChangeLog: >> >> * tree-vect-patterns.cc (vect_recog_bit_insert_pattern): Attempt to >> fold shifted value. >> --- >> gcc/tree-vect-patterns.cc | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc >> index 6afd57a50c4..3beda774ec3 100644 >> --- a/gcc/tree-vect-patterns.cc >> +++ b/gcc/tree-vect-patterns.cc >> @@ -2098,9 +2098,11 @@ vect_recog_bit_insert_pattern (vec_info *vinfo, stmt_vec_info stmt_info, >> tree shifted = value; >> if (shift_n) >> { >> + tree shifted_tmp >> + = fold_build2 (LSHIFT_EXPR, container_type, value, shift); >> pattern_stmt >> = gimple_build_assign (vect_recog_temp_ssa_var (container_type), >> - LSHIFT_EXPR, value, shift); >> + shifted_tmp); > > The canonical way would be to use > > gimple_seq stmts = NULL; > shifted = gimple_build (&stmts, LSHIFT_EXPR, container_type, > value, shift); > if (!gimple_seq_empty_p (stmts)) > append_pattern_def_seq (vinfo, stmt_info, > gimple_seq_first_stmt (stmts)); > > That also avoids the spurious val = constant; with your patch. > Thanks for the suggestion! This works well by testing those two cases locally. I searched around, it seems gimple_build doesn't provide one interface for its users to specify a ssa name for the result, previously we use vect_recog_temp_ssa_var for the shifted result, but I think it's trivial. I'll do a full testing further as before and push it if everything goes well. Thanks again. BR, Kewen > OK if that works. > > thanks, > Richard. > >> append_pattern_def_seq (vinfo, stmt_info, pattern_stmt); >> shifted = gimple_get_lhs (pattern_stmt); >> } >> -- >> 2.27.0
diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc index 6afd57a50c4..3beda774ec3 100644 --- a/gcc/tree-vect-patterns.cc +++ b/gcc/tree-vect-patterns.cc @@ -2098,9 +2098,11 @@ vect_recog_bit_insert_pattern (vec_info *vinfo, stmt_vec_info stmt_info, tree shifted = value; if (shift_n) { + tree shifted_tmp + = fold_build2 (LSHIFT_EXPR, container_type, value, shift); pattern_stmt = gimple_build_assign (vect_recog_temp_ssa_var (container_type), - LSHIFT_EXPR, value, shift); + shifted_tmp); append_pattern_def_seq (vinfo, stmt_info, pattern_stmt); shifted = gimple_get_lhs (pattern_stmt); }