Re: [V2 PATCH] Handle bitop with INTEGER_CST in analyze_and_compute_bitop_with_inv_effect.

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Nov 7, 2023 at 7:08 AM liuhongt <hongtao.liu@intel.com> wrote:
>
> analyze_and_compute_bitop_with_inv_effect assumes the first operand is
> loop invariant which is not the case when it's INTEGER_CST.
>
> Bootstrapped and regtseted on x86_64-pc-linux-gnu{-m32,}.
> Ok for trunk?

So this addresses a missed optimization, right?  It seems to me that
even with two SSA names we are only "lucky" when rhs1 is the invariant
one.  So instead of swapping this way I'd do

 unsigned i;
 for (i = 0; i < 2; ++i)
   if (TREE_CODE (match_op[i]) == SSA_NAME
       && ...)
    break; /* found! */

  if (i == 2)
    return NULL_TREE;
  if (i == 0)
    std::swap (match_op[0], match_op[1]);

to also handle a "swapped" pair of SSA names?

> gcc/ChangeLog:
>
>         PR tree-optimization/105735
>         PR tree-optimization/111972
>         * tree-scalar-evolution.cc
>         (analyze_and_compute_bitop_with_inv_effect): Handle bitop with
>         INTEGER_CST.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/i386/pr105735-3.c: New test.
> ---
>  gcc/testsuite/gcc.target/i386/pr105735-3.c | 87 ++++++++++++++++++++++
>  gcc/tree-scalar-evolution.cc               |  3 +
>  2 files changed, 90 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr105735-3.c
>
> diff --git a/gcc/testsuite/gcc.target/i386/pr105735-3.c b/gcc/testsuite/gcc.target/i386/pr105735-3.c
> new file mode 100644
> index 00000000000..9e268a1a997
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr105735-3.c
> @@ -0,0 +1,87 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fdump-tree-sccp-details" } */
> +/* { dg-final { scan-tree-dump-times {final value replacement} 8 "sccp" } } */
> +
> +unsigned int
> +__attribute__((noipa))
> +foo (unsigned int tmp)
> +{
> +  for (int bit = 0; bit < 64; bit++)
> +    tmp &= 11304;
> +  return tmp;
> +}
> +
> +unsigned int
> +__attribute__((noipa))
> +foo1 (unsigned int tmp)
> +{
> +  for (int bit = 63; bit >= 0; bit -=3)
> +    tmp &= 11304;
> +  return tmp;
> +}
> +
> +unsigned int
> +__attribute__((noipa))
> +foo2 (unsigned int tmp)
> +{
> +  for (int bit = 0; bit < 64; bit++)
> +    tmp |= 11304;
> +  return tmp;
> +}
> +
> +unsigned int
> +__attribute__((noipa))
> +foo3 (unsigned int tmp)
> +{
> +  for (int bit = 63; bit >= 0; bit -=3)
> +    tmp |= 11304;
> +  return tmp;
> +}
> +
> +unsigned int
> +__attribute__((noipa))
> +foo4 (unsigned int tmp)
> +{
> +  for (int bit = 0; bit < 64; bit++)
> +    tmp ^= 11304;
> +  return tmp;
> +}
> +
> +unsigned int
> +__attribute__((noipa))
> +foo5 (unsigned int tmp)
> +{
> +  for (int bit = 0; bit < 63; bit++)
> +    tmp ^= 11304;
> +  return tmp;
> +}
> +
> +unsigned int
> +__attribute__((noipa))
> +f (unsigned int tmp, int bit)
> +{
> +  unsigned int res = tmp;
> +  for (int i = 0; i < bit; i++)
> +    res &= 11304;
> +  return res;
> +}
> +
> +unsigned int
> +__attribute__((noipa))
> +f1 (unsigned int tmp, int bit)
> +{
> +  unsigned int res = tmp;
> +  for (int i = 0; i < bit; i++)
> +    res |= 11304;
> +  return res;
> +}
> +
> +unsigned int
> +__attribute__((noipa))
> +f2 (unsigned int tmp, int bit)
> +{
> +  unsigned int res = tmp;
> +  for (int i = 0; i < bit; i++)
> +    res ^= 11304;
> +  return res;
> +}
> diff --git a/gcc/tree-scalar-evolution.cc b/gcc/tree-scalar-evolution.cc
> index 70b17c5bca1..f61277c32df 100644
> --- a/gcc/tree-scalar-evolution.cc
> +++ b/gcc/tree-scalar-evolution.cc
> @@ -3689,6 +3689,9 @@ analyze_and_compute_bitop_with_inv_effect (class loop* loop, tree phidef,
>    match_op[0] = gimple_assign_rhs1 (def);
>    match_op[1] = gimple_assign_rhs2 (def);
>
> +  if (expr_invariant_in_loop_p (loop, match_op[1]))
> +    std::swap (match_op[0], match_op[1]);
> +
>    if (TREE_CODE (match_op[1]) != SSA_NAME
>        || !expr_invariant_in_loop_p (loop, match_op[0])
>        || !(header_phi = dyn_cast <gphi *> (SSA_NAME_DEF_STMT (match_op[1])))
> --
> 2.31.1
>