diff mbox series

vect: Try folding first for shifted value generation [PR107240]

Message ID 31c05be7-64bf-8d93-934c-63262e082e68@linux.ibm.com
State Accepted
Headers
Series vect: Try folding first for shifted value generation [PR107240] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Kewen.Lin Oct. 19, 2022, 3:18 a.m. UTC 
  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(-)

--
2.27.0

Comments

Richard Biener Oct. 19, 2022, 7:43 a.m. UTC | #1 
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
Kewen.Lin Oct. 19, 2022, 8:51 a.m. UTC | #2 
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 mbox series

Patch

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);
     }