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
next 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).