* [PATCH] Clean-up EH after strlen transformation (PR tree-optimization/83593).
@ 2018-01-03 12:27 Martin Liška
2018-01-03 12:50 ` Marc Glisse
2018-01-03 12:50 ` Jakub Jelinek
0 siblings, 2 replies; 6+ messages in thread
From: Martin Liška @ 2018-01-03 12:27 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 1038 bytes --]
Hi.
Strlen pass does following transformation:
Optimizing: _7 = *ju_5(D);
into: _7 = 0;
which leads to need of removal of EH for the gimple statement. That's done via maybe_clean_or_replace_eh_stmt
and then we need to call gimple_purge_dead_eh_edges. Last question I have is whether it's also needed to
return TODO_cleanup_cfg or not?
Patch as is can bootstrap on ppc64le-redhat-linux and survives regression tests.
Martin
gcc/ChangeLog:
2018-01-03 Martin Liska <mliska@suse.cz>
PR tree-optimization/83593
* tree-ssa-strlen.c (strlen_check_and_optimize_stmt): Clean-up
EH gimple statements.
(strlen_dom_walker::before_dom_children): Call
gimple_purge_dead_eh_edges.
gcc/testsuite/ChangeLog:
2018-01-03 Martin Liska <mliska@suse.cz>
PR tree-optimization/83593
* gcc.dg/pr83593.c: New test.
---
gcc/testsuite/gcc.dg/pr83593.c | 15 +++++++++++++++
gcc/tree-ssa-strlen.c | 28 +++++++++++++++++++++++++---
2 files changed, 40 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr83593.c
[-- Attachment #2: 0001-Clean-up-EH-after-strlen-transformation-PR-tree-opti.patch --]
[-- Type: text/x-patch, Size: 2915 bytes --]
diff --git a/gcc/testsuite/gcc.dg/pr83593.c b/gcc/testsuite/gcc.dg/pr83593.c
new file mode 100644
index 00000000000..eddecc0606a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr83593.c
@@ -0,0 +1,15 @@
+/* PR tree-optimization/83593 */
+/* { dg-options "-O2 -fno-tree-dominator-opts -fnon-call-exceptions -fno-tree-pre -fexceptions -fno-code-hoisting -fno-tree-fre" } */
+
+void
+hr (int *ed, signed char *ju)
+{
+ int kc;
+ {
+ int xj;
+ int *q2 = (*ed == 0) ? &xj : &kc;
+
+ *ju = 0;
+ kc = *ju;
+ }
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index be6ab9f1e1b..293aeceacce 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-iterator.h"
#include "gimplify-me.h"
#include "expr.h"
+#include "tree-cfg.h"
#include "tree-dfa.h"
#include "domwalk.h"
#include "tree-ssa-alias.h"
@@ -3051,10 +3052,12 @@ fold_strstr_to_strncmp (tree rhs1, tree rhs2, gimple *stmt)
}
/* Attempt to check for validity of the performed access a single statement
- at *GSI using string length knowledge, and to optimize it. */
+ 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. */
static bool
-strlen_check_and_optimize_stmt (gimple_stmt_iterator *gsi)
+strlen_check_and_optimize_stmt (gimple_stmt_iterator *gsi, bool *cleanup_eh)
{
gimple *stmt = gsi_stmt (*gsi);
@@ -3201,11 +3204,25 @@ strlen_check_and_optimize_stmt (gimple_stmt_iterator *gsi)
if (w1 == w2
&& si->full_string_p)
{
+ if (dump_file && (dump_flags & TDF_DETAILS) != 0)
+ {
+ fprintf (dump_file, "Optimizing: ");
+ print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
+ }
+
/* Reading the final '\0' character. */
tree zero = build_int_cst (TREE_TYPE (lhs), 0);
gimple_set_vuse (stmt, NULL_TREE);
gimple_assign_set_rhs_from_tree (gsi, zero);
+ *cleanup_eh = maybe_clean_or_replace_eh_stmt (stmt,
+ stmt);
update_stmt (gsi_stmt (*gsi));
+
+ if (dump_file && (dump_flags & TDF_DETAILS) != 0)
+ {
+ fprintf (dump_file, "into: ");
+ print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
+ }
}
else if (w1 > w2)
{
@@ -3399,11 +3416,16 @@ strlen_dom_walker::before_dom_children (basic_block bb)
}
}
+ bool cleanup_eh = false;
+
/* Attempt to optimize individual statements. */
for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
- if (strlen_check_and_optimize_stmt (&gsi))
+ if (strlen_check_and_optimize_stmt (&gsi, &cleanup_eh))
gsi_next (&gsi);
+ if (cleanup_eh)
+ gimple_purge_dead_eh_edges (bb);
+
bb->aux = stridx_to_strinfo;
if (vec_safe_length (stridx_to_strinfo) && !strinfo_shared ())
(*stridx_to_strinfo)[0] = (strinfo *) bb;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Clean-up EH after strlen transformation (PR tree-optimization/83593).
2018-01-03 12:27 [PATCH] Clean-up EH after strlen transformation (PR tree-optimization/83593) Martin Liška
2018-01-03 12:50 ` Marc Glisse
@ 2018-01-03 12:50 ` Jakub Jelinek
2018-01-03 13:07 ` Martin Liška
1 sibling, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2018-01-03 12:50 UTC (permalink / raw)
To: Martin Liška; +Cc: gcc-patches
On Wed, Jan 03, 2018 at 01:27:01PM +0100, Martin Liška wrote:
> /* Reading the final '\0' character. */
> tree zero = build_int_cst (TREE_TYPE (lhs), 0);
> gimple_set_vuse (stmt, NULL_TREE);
> gimple_assign_set_rhs_from_tree (gsi, zero);
> + *cleanup_eh = maybe_clean_or_replace_eh_stmt (stmt,
> + stmt);
The second stmt should be gsi_stmt (*gsi) just in case
gimple_assign_set_rhs_from_tree can't modify in-place, and probably you need
*cleanup_eh
= maybe_clean_or_replace_eh_stmt (stmt,
gsi_stmt (*gsi));
then to get formatting right.
> update_stmt (gsi_stmt (*gsi));
> +
> + if (dump_file && (dump_flags & TDF_DETAILS) != 0)
> + {
> + fprintf (dump_file, "into: ");
> + print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
Again, should be gsi_stmt (*gsi);. Or do:
stmt = gsi_stmt (*gsi);
above update_stmt. As stmt is used several times later, I think that is my
preference (though, in maybe_clean_or_replace_eh_stmt call you IMHO still want
gsi_stmt repeated).
> + }
> }
> else if (w1 > w2)
> {
> @@ -3399,11 +3416,16 @@ strlen_dom_walker::before_dom_children (basic_block bb)
> }
> }
>
> + bool cleanup_eh = false;
> +
> /* Attempt to optimize individual statements. */
> for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
> - if (strlen_check_and_optimize_stmt (&gsi))
> + if (strlen_check_and_optimize_stmt (&gsi, &cleanup_eh))
> gsi_next (&gsi);
>
> + if (cleanup_eh)
> + gimple_purge_dead_eh_edges (bb);
If gimple_purge_dead_eh_edges returns true, you want to arrange for the
pass to return TODO_cleanup_cfg (probably needs to use some global static
variable to propagate that).
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Clean-up EH after strlen transformation (PR tree-optimization/83593).
2018-01-03 12:27 [PATCH] Clean-up EH after strlen transformation (PR tree-optimization/83593) Martin Liška
@ 2018-01-03 12:50 ` Marc Glisse
2018-01-03 12:56 ` Martin Liška
2018-01-03 12:50 ` Jakub Jelinek
1 sibling, 1 reply; 6+ messages in thread
From: Marc Glisse @ 2018-01-03 12:50 UTC (permalink / raw)
To: Martin Liška; +Cc: gcc-patches, Jakub Jelinek
On Wed, 3 Jan 2018, Martin Liška wrote:
+ *cleanup_eh = maybe_clean_or_replace_eh_stmt (stmt,
+ stmt);
Do you mean *cleanup_eh |= ... ?
--
Marc Glisse
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Clean-up EH after strlen transformation (PR tree-optimization/83593).
2018-01-03 12:50 ` Marc Glisse
@ 2018-01-03 12:56 ` Martin Liška
0 siblings, 0 replies; 6+ messages in thread
From: Martin Liška @ 2018-01-03 12:56 UTC (permalink / raw)
To: gcc-patches, Marc Glisse; +Cc: Jakub Jelinek
On 01/03/2018 01:49 PM, Marc Glisse wrote:
> On Wed, 3 Jan 2018, Martin Liška wrote:
>
> +Â Â Â Â Â Â Â Â Â Â Â *cleanup_eh = maybe_clean_or_replace_eh_stmt (stmt,
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â stmt);
>
> Do you mean *cleanup_eh |= ... ?
>
Yes. Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Clean-up EH after strlen transformation (PR tree-optimization/83593).
2018-01-03 12:50 ` Jakub Jelinek
@ 2018-01-03 13:07 ` Martin Liška
2018-01-03 13:13 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Martin Liška @ 2018-01-03 13:07 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 390 bytes --]
On 01/03/2018 01:50 PM, Jakub Jelinek wrote:
> If gimple_purge_dead_eh_edges returns true, you want to arrange for the
> pass to return TODO_cleanup_cfg (probably needs to use some global static
> variable to propagate that).
>
> Jakub
Hi.
Sending v2. I'm suggesting to propagate that in strlen_dom_walker::m_cleanup_cfg.
I've just triggered tests.
Ready to install after it finishes?
[-- Attachment #2: 0001-Clean-up-EH-after-strlen-transformation-PR-tree-opti.patch --]
[-- Type: text/x-patch, Size: 5209 bytes --]
From 217982bae6969865051f12798e4da444dd4aaa3f Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 3 Jan 2018 12:15:40 +0100
Subject: [PATCH] Clean-up EH after strlen transformation (PR
tree-optimization/83593).
gcc/ChangeLog:
2018-01-03 Martin Liska <mliska@suse.cz>
PR tree-optimization/83593
* tree-ssa-strlen.c (strlen_check_and_optimize_stmt): Clean-up
EH gimple statements.
(strlen_dom_walker::before_dom_children): Call
gimple_purge_dead_eh_edges.
(pass_strlen::execute): Return TODO_cleanup_cfg if needed.
gcc/testsuite/ChangeLog:
2018-01-03 Martin Liska <mliska@suse.cz>
PR tree-optimization/83593
* gcc.dg/pr83593.c: New test.
---
gcc/testsuite/gcc.dg/pr83593.c | 15 +++++++++++++
gcc/tree-ssa-strlen.c | 48 ++++++++++++++++++++++++++++++++++++------
2 files changed, 56 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr83593.c
diff --git a/gcc/testsuite/gcc.dg/pr83593.c b/gcc/testsuite/gcc.dg/pr83593.c
new file mode 100644
index 00000000000..eddecc0606a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr83593.c
@@ -0,0 +1,15 @@
+/* PR tree-optimization/83593 */
+/* { dg-options "-O2 -fno-tree-dominator-opts -fnon-call-exceptions -fno-tree-pre -fexceptions -fno-code-hoisting -fno-tree-fre" } */
+
+void
+hr (int *ed, signed char *ju)
+{
+ int kc;
+ {
+ int xj;
+ int *q2 = (*ed == 0) ? &xj : &kc;
+
+ *ju = 0;
+ kc = *ju;
+ }
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index be6ab9f1e1b..7ad6ff8228c 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-iterator.h"
#include "gimplify-me.h"
#include "expr.h"
+#include "tree-cfg.h"
#include "tree-dfa.h"
#include "domwalk.h"
#include "tree-ssa-alias.h"
@@ -3051,10 +3052,12 @@ fold_strstr_to_strncmp (tree rhs1, tree rhs2, gimple *stmt)
}
/* Attempt to check for validity of the performed access a single statement
- at *GSI using string length knowledge, and to optimize it. */
+ 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. */
static bool
-strlen_check_and_optimize_stmt (gimple_stmt_iterator *gsi)
+strlen_check_and_optimize_stmt (gimple_stmt_iterator *gsi, bool *cleanup_eh)
{
gimple *stmt = gsi_stmt (*gsi);
@@ -3201,11 +3204,27 @@ strlen_check_and_optimize_stmt (gimple_stmt_iterator *gsi)
if (w1 == w2
&& si->full_string_p)
{
+ if (dump_file && (dump_flags & TDF_DETAILS) != 0)
+ {
+ fprintf (dump_file, "Optimizing: ");
+ print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
+ }
+
/* Reading the final '\0' character. */
tree zero = build_int_cst (TREE_TYPE (lhs), 0);
gimple_set_vuse (stmt, NULL_TREE);
gimple_assign_set_rhs_from_tree (gsi, zero);
- update_stmt (gsi_stmt (*gsi));
+ *cleanup_eh
+ |= maybe_clean_or_replace_eh_stmt (stmt,
+ gsi_stmt (*gsi));
+ stmt = gsi_stmt (*gsi);
+ update_stmt (stmt);
+
+ if (dump_file && (dump_flags & TDF_DETAILS) != 0)
+ {
+ fprintf (dump_file, "into: ");
+ print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
+ }
}
else if (w1 > w2)
{
@@ -3318,10 +3337,16 @@ do_invalidate (basic_block dombb, gimple *phi, bitmap visited, int *count)
class strlen_dom_walker : public dom_walker
{
public:
- strlen_dom_walker (cdi_direction direction) : dom_walker (direction) {}
+ strlen_dom_walker (cdi_direction direction)
+ : dom_walker (direction), m_cleanup_cfg (false)
+ {}
virtual edge before_dom_children (basic_block);
virtual void after_dom_children (basic_block);
+
+ /* Flag that will trigger TODO_cleanup_cfg to be returned in strlen
+ execute function. */
+ bool m_cleanup_cfg;
};
/* Callback for walk_dominator_tree. Attempt to optimize various
@@ -3399,11 +3424,19 @@ strlen_dom_walker::before_dom_children (basic_block bb)
}
}
+ bool cleanup_eh = false;
+
/* Attempt to optimize individual statements. */
for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
- if (strlen_check_and_optimize_stmt (&gsi))
+ if (strlen_check_and_optimize_stmt (&gsi, &cleanup_eh))
gsi_next (&gsi);
+ if (cleanup_eh)
+ {
+ gimple_purge_dead_eh_edges (bb);
+ m_cleanup_cfg = true;
+ }
+
bb->aux = stridx_to_strinfo;
if (vec_safe_length (stridx_to_strinfo) && !strinfo_shared ())
(*stridx_to_strinfo)[0] = (strinfo *) bb;
@@ -3477,7 +3510,8 @@ pass_strlen::execute (function *fun)
/* String length optimization is implemented as a walk of the dominator
tree and a forward walk of statements within each block. */
- strlen_dom_walker (CDI_DOMINATORS).walk (fun->cfg->x_entry_block_ptr);
+ strlen_dom_walker walker (CDI_DOMINATORS);
+ walker.walk (fun->cfg->x_entry_block_ptr);
ssa_ver_to_stridx.release ();
strinfo_pool.release ();
@@ -3498,7 +3532,7 @@ pass_strlen::execute (function *fun)
strlen_to_stridx = NULL;
}
- return 0;
+ return walker.m_cleanup_cfg ? TODO_cleanup_cfg : 0;
}
} // anon namespace
--
2.14.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Clean-up EH after strlen transformation (PR tree-optimization/83593).
2018-01-03 13:07 ` Martin Liška
@ 2018-01-03 13:13 ` Jakub Jelinek
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2018-01-03 13:13 UTC (permalink / raw)
To: Martin Liška; +Cc: gcc-patches
On Wed, Jan 03, 2018 at 02:07:36PM +0100, Martin Liška wrote:
> 2018-01-03 Martin Liska <mliska@suse.cz>
>
> PR tree-optimization/83593
> * tree-ssa-strlen.c (strlen_check_and_optimize_stmt): Clean-up
> EH gimple statements.
> (strlen_dom_walker::before_dom_children): Call
> gimple_purge_dead_eh_edges.
> (pass_strlen::execute): Return TODO_cleanup_cfg if needed.
The ChangeLog entry doesn't contain all the changes, like:
> @@ -39,6 +39,7 @@ along with GCC; see the file COPYING3. If not see
> #include "gimple-iterator.h"
> #include "gimplify-me.h"
> #include "expr.h"
> +#include "tree-cfg.h"
> #include "tree-dfa.h"
> #include "domwalk.h"
> #include "tree-ssa-alias.h"
the above one.
> static bool
> -strlen_check_and_optimize_stmt (gimple_stmt_iterator *gsi)
> +strlen_check_and_optimize_stmt (gimple_stmt_iterator *gsi, bool *cleanup_eh)
This one (i.e. addition of a new argument).
> @@ -3318,10 +3337,16 @@ do_invalidate (basic_block dombb, gimple *phi, bitmap visited, int *count)
> class strlen_dom_walker : public dom_walker
> {
> public:
> - strlen_dom_walker (cdi_direction direction) : dom_walker (direction) {}
> + strlen_dom_walker (cdi_direction direction)
> + : dom_walker (direction), m_cleanup_cfg (false)
> + {}
This one.
>
> virtual edge before_dom_children (basic_block);
> virtual void after_dom_children (basic_block);
> +
> + /* Flag that will trigger TODO_cleanup_cfg to be returned in strlen
> + execute function. */
> + bool m_cleanup_cfg;
This one too.
> + bool cleanup_eh = false;
> +
> /* Attempt to optimize individual statements. */
> for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); )
> - if (strlen_check_and_optimize_stmt (&gsi))
> + if (strlen_check_and_optimize_stmt (&gsi, &cleanup_eh))
> gsi_next (&gsi);
And the fact that strlen_check_and_optimize_stmt caller has been adjusted.
>
> + if (cleanup_eh)
> + {
> + gimple_purge_dead_eh_edges (bb);
> + m_cleanup_cfg = true;
> + }
This should be
if (cleanup_eh && gimple_purge_dead_eh_edges (bb))
m_cleanup_cfg = true;
Ok with that change and updated ChangeLog entry if it passes
bootstrap/regtest.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-03 13:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-03 12:27 [PATCH] Clean-up EH after strlen transformation (PR tree-optimization/83593) Martin Liška
2018-01-03 12:50 ` Marc Glisse
2018-01-03 12:56 ` Martin Liška
2018-01-03 12:50 ` Jakub Jelinek
2018-01-03 13:07 ` Martin Liška
2018-01-03 13:13 ` Jakub Jelinek
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).