public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <rguenther@suse.de>
To: gcc-patches@gcc.gnu.org
Cc: Jakub Jelinek <jakub@redhat.com>
Subject: [PATCH] tree-optimization/109237 - last_stmt is possibly slow
Date: Wed, 22 Mar 2023 12:29:52 +0000 (UTC)	[thread overview]
Message-ID: <20230322122952.inFbDZKg2mCVqxD_sLsQAkgxwWKQJ6Cq1XTvtMy1GMU@z> (raw)

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

             reply	other threads:[~2023-03-22 12:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-22 12:29 Richard Biener [this message]
     [not found] <90515.123032208295400168@us-mta-397.us.mimecast.lan>
2023-03-22 12:38 ` Jakub Jelinek
     [not found] <20230322123020.B9718385B533@sourceware.org>
2023-03-22 16:50 ` Bernhard Reutner-Fischer
     [not found] <20230322123006.A480C3858296@sourceware.org>
2023-03-26 18:00 ` Jeff Law

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=20230322122952.inFbDZKg2mCVqxD_sLsQAkgxwWKQJ6Cq1XTvtMy1GMU@z \
    --to=rguenther@suse.de \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    /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).