[parent not found: <20230322123006.A480C3858296@sourceware.org>]
* Re: [PATCH] tree-optimization/109237 - last_stmt is possibly slow
[not found] <20230322123006.A480C3858296@sourceware.org>
@ 2023-03-26 18:00 ` Jeff Law
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Law @ 2023-03-26 18:00 UTC (permalink / raw)
To: Richard Biener, gcc-patches; +Cc: Jakub Jelinek
On 3/22/23 06:29, Richard Biener via Gcc-patches wrote:
> Most uses of last_stmt are interested in control transfer stmts
> and for the testcase gimple_purge_dead_eh_edges shows up in
> the profile. But last_stmt looks past trailing debug stmts but
> those would be rejected by GIMPLEs verify_flow_info. The following
> adds possible_ctrl_stmt besides last_stmt which does not look
> past trailing debug stmts and adjusts gimple_purge_dead_eh_edges.
>
> I've put checking code into possible_ctrl_stmt that it will not
> miss a control statement if the real last stmt is a debug stmt.
>
> The alternative would be to change last_stmt, explicitely introducing
> last_nondebug_stmt. I remember we chickened out and made last_stmt
> conservative here but not anticipating the compile-time issues this
> creates. I count 227 last_stmt and 12 last_and_only_stmt uses.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> Any opinions? I probably lean towards s/last_stmt/last_nondebug_stmt/
> in next stage1 and then adding last_stmt and changing some
> uses back - through for maintainance that's going to be a
> nightmare (or maybe not, a "wrong" last_stmt should be safe to
> backport and a last_nondebug_stmt will fail to build).
Sounds quite sensible to me. 227+12 isn't terrible and I bet the vast
majority, should be safe for last_nondebug_stmt.
>
> Richard.
>
> PR tree-optimization/109237
> * tree-cfg.h (possible_ctrl_stmt): New function returning
> the last stmt not skipping debug stmts.
> (gimple_purge_dead_eh_edges): Use it.
OK
jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <90515.123032208295400168@us-mta-397.us.mimecast.lan>]
* Re: [PATCH] tree-optimization/109237 - last_stmt is possibly slow
[not found] <90515.123032208295400168@us-mta-397.us.mimecast.lan>
@ 2023-03-22 12:38 ` Jakub Jelinek
0 siblings, 0 replies; 4+ messages in thread
From: Jakub Jelinek @ 2023-03-22 12:38 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
On Wed, Mar 22, 2023 at 12:29:52PM +0000, Richard Biener wrote:
> Most uses of last_stmt are interested in control transfer stmts
> and for the testcase gimple_purge_dead_eh_edges shows up in
> the profile. But last_stmt looks past trailing debug stmts but
> those would be rejected by GIMPLEs verify_flow_info. The following
> adds possible_ctrl_stmt besides last_stmt which does not look
> past trailing debug stmts and adjusts gimple_purge_dead_eh_edges.
>
> I've put checking code into possible_ctrl_stmt that it will not
> miss a control statement if the real last stmt is a debug stmt.
>
> The alternative would be to change last_stmt, explicitely introducing
> last_nondebug_stmt. I remember we chickened out and made last_stmt
> conservative here but not anticipating the compile-time issues this
> creates. I count 227 last_stmt and 12 last_and_only_stmt uses.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
>
> Any opinions? I probably lean towards s/last_stmt/last_nondebug_stmt/
> in next stage1 and then adding last_stmt and changing some
> uses back - through for maintainance that's going to be a
> nightmare (or maybe not, a "wrong" last_stmt should be safe to
> backport and a last_nondebug_stmt will fail to build).
>
> Richard.
>
> PR tree-optimization/109237
> * tree-cfg.h (possible_ctrl_stmt): New function returning
> the last stmt not skipping debug stmts.
> (gimple_purge_dead_eh_edges): Use it.
LGTM. But finding out which of those 227+12 calls want to skip
debug stmts and which don't will be a nightmare...
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] tree-optimization/109237 - last_stmt is possibly slow
@ 2023-03-22 12:29 Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2023-03-22 12:29 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek
Most uses of last_stmt are interested in control transfer stmts
and for the testcase gimple_purge_dead_eh_edges shows up in
the profile. But last_stmt looks past trailing debug stmts but
those would be rejected by GIMPLEs verify_flow_info. The following
adds possible_ctrl_stmt besides last_stmt which does not look
past trailing debug stmts and adjusts gimple_purge_dead_eh_edges.
I've put checking code into possible_ctrl_stmt that it will not
miss a control statement if the real last stmt is a debug stmt.
The alternative would be to change last_stmt, explicitely introducing
last_nondebug_stmt. I remember we chickened out and made last_stmt
conservative here but not anticipating the compile-time issues this
creates. I count 227 last_stmt and 12 last_and_only_stmt uses.
Bootstrapped and tested on x86_64-unknown-linux-gnu.
Any opinions? I probably lean towards s/last_stmt/last_nondebug_stmt/
in next stage1 and then adding last_stmt and changing some
uses back - through for maintainance that's going to be a
nightmare (or maybe not, a "wrong" last_stmt should be safe to
backport and a last_nondebug_stmt will fail to build).
Richard.
PR tree-optimization/109237
* tree-cfg.h (possible_ctrl_stmt): New function returning
the last stmt not skipping debug stmts.
(gimple_purge_dead_eh_edges): Use it.
---
gcc/tree-cfg.cc | 22 +++++++++++++++++++++-
gcc/tree-cfg.h | 1 +
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index a9fcc7fd050..e167596209b 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -2947,6 +2947,26 @@ first_non_label_stmt (basic_block bb)
return !gsi_end_p (i) ? gsi_stmt (i) : NULL;
}
+/* Return the last statement of a basic block BB that's possibly control
+ altering. Compared to last_stmt this will return a debug stmt if that
+ is the last stmt. */
+
+gimple *
+possible_ctrl_stmt (basic_block bb)
+{
+ gimple_stmt_iterator i = gsi_last_bb (bb);
+ if (gsi_end_p (i))
+ return NULL;
+ if (flag_checking && is_gimple_debug (gsi_stmt (i)))
+ {
+ /* Verify that if the real last stmt is a debug stmt the
+ last non-debug stmt isn't control altering. */
+ gimple *last = last_stmt (bb);
+ gcc_assert (!last || !stmt_ends_bb_p (last));
+ }
+ return gsi_stmt (i);
+}
+
/* Return the last statement in basic block BB. */
gimple *
@@ -8990,7 +9010,7 @@ gimple_purge_dead_eh_edges (basic_block bb)
bool changed = false;
edge e;
edge_iterator ei;
- gimple *stmt = last_stmt (bb);
+ gimple *stmt = possible_ctrl_stmt (bb);
if (stmt && stmt_can_throw_internal (cfun, stmt))
return false;
diff --git a/gcc/tree-cfg.h b/gcc/tree-cfg.h
index 9b56a68fe9d..7c6a9a4f16b 100644
--- a/gcc/tree-cfg.h
+++ b/gcc/tree-cfg.h
@@ -61,6 +61,7 @@ extern bool assert_unreachable_fallthru_edge_p (edge);
extern void delete_tree_cfg_annotations (function *);
extern gphi *get_virtual_phi (basic_block);
extern gimple *first_stmt (basic_block);
+extern gimple *possible_ctrl_stmt (basic_block);
extern gimple *last_stmt (basic_block);
extern gimple *last_and_only_stmt (basic_block);
extern bool verify_gimple_in_seq (gimple_seq, bool = true);
--
2.35.3
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-26 18:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20230322123020.B9718385B533@sourceware.org>
2023-03-22 16:50 ` [PATCH] tree-optimization/109237 - last_stmt is possibly slow Bernhard Reutner-Fischer
[not found] <20230322123006.A480C3858296@sourceware.org>
2023-03-26 18:00 ` Jeff Law
[not found] <90515.123032208295400168@us-mta-397.us.mimecast.lan>
2023-03-22 12:38 ` Jakub Jelinek
2023-03-22 12:29 Richard Biener
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).