public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Richard Biener <rguenther@suse.de>, Jeff Law <law@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] strlen: Return TODO_update_address_taken when memcmp has been optimized [PR96271]
Date: Fri, 15 Jan 2021 19:32:35 +0100	[thread overview]
Message-ID: <20210115183235.GM1034503@tucnak> (raw)

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


             reply	other threads:[~2021-01-15 18:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-15 18:32 Jakub Jelinek [this message]
2021-01-15 19:48 ` Richard Biener
2021-01-15 19:57   ` Jakub Jelinek
2021-01-15 20:16     ` Richard Biener
2021-01-15 20:31       ` Jakub Jelinek
2021-01-15 21:02         ` Jakub Jelinek
2021-01-16  1:00           ` Jakub Jelinek
2021-01-16  7:52             ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210115183235.GM1034503@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=law@redhat.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).