[PATCH] Improve rewrite_to_defined_overflow for lhs already the correct type

[PATCH] Improve rewrite_to_defined_overflow for lhs already the correct type

[Andrew Pinski]

This improves rewrite_to_defined_overflow slightly if we already
have an unsigned type. The only place where this seems to show up
is ifcombine. It removes one extra statement which gets added and
then later on removed.

OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.

gcc/ChangeLog:

	PR tree-optimization/111276
	* gimple-fold.cc (rewrite_to_defined_overflow): Don't
	add a new lhs if we already the unsigned type.
---
 gcc/gimple-fold.cc | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index fd01810581a..2fcafeada37 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -8721,10 +8721,19 @@ rewrite_to_defined_overflow (gimple *stmt, bool in_place /* = false */)
 	op = gimple_convert (&stmts, type, op);
 	gimple_set_op (stmt, i, op);
       }
-  gimple_assign_set_lhs (stmt, make_ssa_name (type, stmt));
+  bool needs_cast_back = false;
+  if (!useless_type_conversion_p (type, TREE_TYPE (lhs)))
+    {
+      gimple_assign_set_lhs (stmt, make_ssa_name (type, stmt));
+      needs_cast_back = true;
+    }
+
   if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
     gimple_assign_set_rhs_code (stmt, PLUS_EXPR);
-  gimple_set_modified (stmt, true);
+
+  if (needs_cast_back || stmts)
+    gimple_set_modified (stmt, true);
+
   if (in_place)
     {
       gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
@@ -8734,6 +8743,10 @@ rewrite_to_defined_overflow (gimple *stmt, bool in_place /* = false */)
     }
   else
     gimple_seq_add_stmt (&stmts, stmt);
+
+  if (!needs_cast_back)
+    return stmts;
+
   gimple *cvt = gimple_build_assign (lhs, NOP_EXPR, gimple_assign_lhs (stmt));
   if (in_place)
     {
-- 
2.31.1

Re: [PATCH] Improve rewrite_to_defined_overflow for lhs already the correct type

[Richard Biener]

On Sun, Sep 3, 2023 at 6:19 PM Andrew Pinski via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This improves rewrite_to_defined_overflow slightly if we already
> have an unsigned type. The only place where this seems to show up
> is ifcombine. It removes one extra statement which gets added and
> then later on removed.

What specific case is that?  It sounds like we call the function when
it isn't needed?  I also think that refactoring to a special case when
the LHS type already is OK will result in better code in the end.

Richard.

> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
>
> gcc/ChangeLog:
>
>         PR tree-optimization/111276
>         * gimple-fold.cc (rewrite_to_defined_overflow): Don't
>         add a new lhs if we already the unsigned type.
> ---
>  gcc/gimple-fold.cc | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index fd01810581a..2fcafeada37 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -8721,10 +8721,19 @@ rewrite_to_defined_overflow (gimple *stmt, bool in_place /* = false */)
>         op = gimple_convert (&stmts, type, op);
>         gimple_set_op (stmt, i, op);
>        }
> -  gimple_assign_set_lhs (stmt, make_ssa_name (type, stmt));
> +  bool needs_cast_back = false;
> +  if (!useless_type_conversion_p (type, TREE_TYPE (lhs)))
> +    {
> +      gimple_assign_set_lhs (stmt, make_ssa_name (type, stmt));
> +      needs_cast_back = true;
> +    }
> +
>    if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>      gimple_assign_set_rhs_code (stmt, PLUS_EXPR);
> -  gimple_set_modified (stmt, true);
> +
> +  if (needs_cast_back || stmts)
> +    gimple_set_modified (stmt, true);
> +
>    if (in_place)
>      {
>        gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
> @@ -8734,6 +8743,10 @@ rewrite_to_defined_overflow (gimple *stmt, bool in_place /* = false */)
>      }
>    else
>      gimple_seq_add_stmt (&stmts, stmt);
> +
> +  if (!needs_cast_back)
> +    return stmts;
> +
>    gimple *cvt = gimple_build_assign (lhs, NOP_EXPR, gimple_assign_lhs (stmt));
>    if (in_place)
>      {
> --
> 2.31.1
>