[PATCH] strlen: Return TODO_update_address_taken when memcmp has been optimized [PR96271]

[Jakub Jelinek]

Hi!

On the following testcase, handle_builtin_memcmp in the strlen pass folds
the memcmp into comparison of two MEM_REFs.  But nothing triggers updating
of addressable vars afterwards, so even when the parameters are no longer
address taken, we force the parameters to stack and back anyway.

The following patch fixes that.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-01-15  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/96271
	* tree-ssa-strlen.c (handle_builtin_memcmp): Add UPDATE_ADDRESS_TAKEN
	argument, set what it points to to true if optimizing memcmp into
	a simple MEM_REF comparison.
	(strlen_check_and_optimize_call): Add UPDATE_ADDRESS_TAKEN argument
	and pass it through to handle_builtin_memcmp.
	(check_and_optimize_stmt): Add UPDATE_ADDRESS_TAKEN argument
	and pass it through to strlen_check_and_optimize_call.
	(strlen_dom_walker): Replace m_cleanup_cfg with todo.
	(strlen_dom_walker::before_dom_children): Adjust for the above change,
	adjust check_and_optimize_stmt caller and or in into todo
	TODO_cleanup_cfg and/or TODO_update_address_taken.
	(printf_strlen_execute): Return todo instead of conditionally
	TODO_cleanup_cfg.

	* gcc.target/i386/pr96271.c: New test.

--- gcc/tree-ssa-strlen.c.jj	2021-01-04 10:25:40.000000000 +0100
+++ gcc/tree-ssa-strlen.c	2021-01-15 15:13:09.847839781 +0100
@@ -3813,7 +3813,7 @@ use_in_zero_equality (tree res, bool exc
    return true when call is transformed, return false otherwise.  */
 
 static bool
-handle_builtin_memcmp (gimple_stmt_iterator *gsi)
+handle_builtin_memcmp (gimple_stmt_iterator *gsi, bool *update_address_taken)
 {
   gcall *stmt = as_a <gcall *> (gsi_stmt (*gsi));
   tree res = gimple_call_lhs (stmt);
@@ -3858,6 +3858,7 @@ handle_builtin_memcmp (gimple_stmt_itera
 						   boolean_type_node,
 						   arg1, arg2));
 	  gimplify_and_update_call_from_tree (gsi, res);
+	  *update_address_taken = true;
 	  return true;
 	}
     }
@@ -5110,7 +5111,7 @@ is_char_type (tree type)
 
 static bool
 strlen_check_and_optimize_call (gimple_stmt_iterator *gsi, bool *zero_write,
-				pointer_query &ptr_qry)
+				bool *update_address_taken, pointer_query &ptr_qry)
 {
   gimple *stmt = gsi_stmt (*gsi);
 
@@ -5179,7 +5180,7 @@ strlen_check_and_optimize_call (gimple_s
 	return false;
       break;
     case BUILT_IN_MEMCMP:
-      if (handle_builtin_memcmp (gsi))
+      if (handle_builtin_memcmp (gsi, update_address_taken))
 	return false;
       break;
     case BUILT_IN_STRCMP:
@@ -5341,12 +5342,13 @@ handle_integral_assign (gimple_stmt_iter
 /* Attempt to check for validity of the performed access a single statement
    at *GSI using string length knowledge, and to optimize it.
    If the given basic block needs clean-up of EH, CLEANUP_EH is set to
-   true.  Return true to let the caller advance *GSI to the next statement
-   in the basic block and false otherwise.  */
+   true.  If it is to update addressables at the end of the pass, set
+   *UPDATE_ADDRESS_TAKEN to true.  Return true to let the caller advance *GSI
+   to the next statement in the basic block and false otherwise.  */
 
 static bool
 check_and_optimize_stmt (gimple_stmt_iterator *gsi, bool *cleanup_eh,
-			 pointer_query &ptr_qry)
+			 bool *update_address_taken, pointer_query &ptr_qry)
 {
   gimple *stmt = gsi_stmt (*gsi);
 
@@ -5356,7 +5358,8 @@ check_and_optimize_stmt (gimple_stmt_ite
 
   if (is_gimple_call (stmt))
     {
-      if (!strlen_check_and_optimize_call (gsi, &zero_write, ptr_qry))
+      if (!strlen_check_and_optimize_call (gsi, &zero_write,
+					   update_address_taken, ptr_qry))
 	return false;
     }
   else if (!flag_optimize_strlen || !strlen_optimize)
@@ -5488,7 +5491,7 @@ public:
     evrp (false),
     ptr_qry (&evrp, &var_cache),
     var_cache (),
-    m_cleanup_cfg (false)
+    todo (0)
   { }
 
   virtual edge before_dom_children (basic_block);
@@ -5503,9 +5506,8 @@ public:
   pointer_query ptr_qry;
   pointer_query::cache_type var_cache;
 
-  /* Flag that will trigger TODO_cleanup_cfg to be returned in strlen
-     execute function.  */
-  bool m_cleanup_cfg;
+  /* TODO_* flags for the pass.  */
+  int todo;
 };
 
 /* Callback for walk_dominator_tree.  Attempt to optimize various
@@ -5586,6 +5588,7 @@ strlen_dom_walker::before_dom_children (
     }
 
   bool cleanup_eh = false;
+  bool update_address_taken = false;
 
   /* Attempt to optimize individual statements.  */
   for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
@@ -5599,12 +5602,15 @@ strlen_dom_walker::before_dom_children (
       /* Reset search depth preformance counter.  */
       ptr_qry.depth = 0;
 
-      if (check_and_optimize_stmt (&gsi, &cleanup_eh, ptr_qry))
+      if (check_and_optimize_stmt (&gsi, &cleanup_eh, &update_address_taken,
+				   ptr_qry))
 	gsi_next (&gsi);
     }
 
   if (cleanup_eh && gimple_purge_dead_eh_edges (bb))
-      m_cleanup_cfg = true;
+    todo |= TODO_cleanup_cfg;
+  if (update_address_taken)
+    todo |= TODO_update_address_taken;
 
   bb->aux = stridx_to_strinfo;
   if (vec_safe_length (stridx_to_strinfo) && !strinfo_shared ())
@@ -5715,7 +5721,7 @@ printf_strlen_execute (function *fun, bo
       loop_optimizer_finalize ();
     }
 
-  return walker.m_cleanup_cfg ? TODO_cleanup_cfg : 0;
+  return walker.todo;
 }
 
 /* This file defines two passes: one for warnings that runs only when
--- gcc/testsuite/gcc.target/i386/pr96271.c.jj	2021-01-15 15:17:42.001780947 +0100
+++ gcc/testsuite/gcc.target/i386/pr96271.c	2021-01-15 15:17:25.447967002 +0100
@@ -0,0 +1,11 @@
+/* PR tree-optimization/96271 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mtune=intel -msse2 -masm=att" } */
+/* { dg-final { scan-assembler "movq\t%xmm0, %r" { target { ! ia32 } } } } */
+/* { dg-final { scan-assembler "movq\t%xmm1, %r" { target { ! ia32 } } } } */
+
+int
+foo (double a, double b)
+{
+  return __builtin_memcmp (&a, &b, sizeof (double)) == 0;
+}

	Jakub