public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Eric Botcazou <botcazou@adacore.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Avoid creating useless debug temporaries
Date: Wed, 26 Apr 2023 11:31:26 +0200	[thread overview]
Message-ID: <4477930.LvFx2qVVIh@fomalhaut> (raw)
In-Reply-To: <CAFiYyc1k8fip63cGNsaj58NKpMCz2fQ_2SXk+5zaG8HCkB4TnA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1249 bytes --]

> probably also helps PR109612 and the other similar PR referenced therein.

Here's a more aggressive patch in this area, but it regresses guality tests, 
for example:

+FAIL: gcc.dg/guality/ipa-sra-1.c   -O2  -DPREVENT_OPTIMIZATION  line 27 k == 
3
+FAIL: gcc.dg/guality/ipa-sra-1.c   -O3 -g  -DPREVENT_OPTIMIZATION  line 27 k 
== 3
+FAIL: gcc.dg/guality/ipa-sra-1.c   -Os  -DPREVENT_OPTIMIZATION  line 27 k == 
3

eric@fomalhaut:~/build/gcc/native> diff -u ipa-sra-1.c.254t.optimized.0 ipa-
sra-1.c.254t.optimized
--- ipa-sra-1.c.254t.optimized.0        2023-04-26 11:12:07.806357325 +0200
+++ ipa-sra-1.c.254t.optimized  2023-04-26 11:24:08.632874257 +0200
@@ -101,7 +101,6 @@
   # DEBUG k => k_5
   # DEBUG BEGIN_STMT
   _1 = get_val1 ();
-  # DEBUG D#6 => k_5
   r_8 = foo.isra (_1);
   # DEBUG r => r_8
   # DEBUG BEGIN_STMT

and I don't understand why yet.


	* tree-ssa-dce.cc (find_debug_expr_decl): New callback.
	(mark_stmt_if_obviously_necessary): Add DECLS parameters.
	<GIMPLE_DEBUG>: Call find_debug_expr_decl on the value of
	DEBUG_BIND statements and record the results in DECLS.
	(find_obviously_necessary_stmts): If DEBUG_BIND statements may be
	present, get rid of those setting an unnecessary DEBUG_EXPR_DECL.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 4023 bytes --]

diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
index 08876bfc1c7..09bbfaca22e 100644
--- a/gcc/tree-ssa-dce.cc
+++ b/gcc/tree-ssa-dce.cc
@@ -191,14 +191,35 @@ mark_operand_necessary (tree op)
 }
 
 
+/* Called via walk_tree, look for DEBUG_EXPR_DECLs and mark them in DATA.  */
+
+static tree
+find_debug_expr_decl (tree *tp, int *walk_subtrees, void *data)
+{
+  auto_bitmap *decls = (auto_bitmap *) data;
+
+  if (TREE_CODE (*tp) == SSA_NAME || IS_TYPE_OR_DECL_P (*tp))
+    {
+      if (TREE_CODE (*tp) == DEBUG_EXPR_DECL)
+	bitmap_set_bit (*decls, DECL_UID (*tp));
+
+      *walk_subtrees = 0;
+    }
+
+  return NULL_TREE;
+}
+
 /* Mark STMT as necessary if it obviously is.  Add it to the worklist if
    it can make other statements necessary.
 
+   If STMT is a DEBUG_BIND, mark the necessary DEBUG_EXPR_DECLs in DECLS.
+
    If AGGRESSIVE is false, control statements are conservatively marked as
    necessary.  */
 
 static void
-mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
+mark_stmt_if_obviously_necessary (gimple *stmt, auto_bitmap *decls,
+				  bool aggressive)
 {
   /* Statements that are implicitly live.  Most function calls, asm
      and return statements are required.  Labels and GIMPLE_BIND nodes
@@ -258,14 +279,28 @@ mark_stmt_if_obviously_necessary (gimple *stmt, bool aggressive)
       }
 
     case GIMPLE_DEBUG:
-      /* Debug temps without a value are not useful.  ??? If we could
-	 easily locate the debug temp bind stmt for a use thereof,
-	 would could refrain from marking all debug temps here, and
-	 mark them only if they're used.  */
-      if (gimple_debug_nonbind_marker_p (stmt)
-	  || !gimple_debug_bind_p (stmt)
-	  || gimple_debug_bind_has_value_p (stmt)
-	  || TREE_CODE (gimple_debug_bind_get_var (stmt)) != DEBUG_EXPR_DECL)
+      if (gimple_debug_bind_p (stmt))
+	{
+	  tree var = gimple_debug_bind_get_var (stmt);
+	  tree val = gimple_debug_bind_get_value (stmt);
+	  bool necessary = false;
+
+	  /* A bind statement for a real variable is always necessary.  */
+	  if (TREE_CODE (var) != DEBUG_EXPR_DECL)
+	    necessary = true;
+
+	  /* A bind statement with a value is necessary for now and we look
+	     into the value to find out necessary DEBUG_EXPR_DECLs.  */
+	  if (val)
+	    {
+	      walk_tree (&val, find_debug_expr_decl, decls, NULL);
+	      necessary = true;
+	    }
+
+	  if (necessary )
+	    mark_stmt_necessary (stmt, false);
+	}
+      else
 	mark_stmt_necessary (stmt, false);
       return;
 
@@ -398,6 +433,7 @@ find_obviously_necessary_stmts (bool aggressive)
   gimple_stmt_iterator gsi;
   edge e;
   gimple *phi, *stmt;
+  auto_bitmap necessary_decls;
   int flags;
 
   FOR_EACH_BB_FN (bb, cfun)
@@ -414,10 +450,35 @@ find_obviously_necessary_stmts (bool aggressive)
 	{
 	  stmt = gsi_stmt (gsi);
 	  gimple_set_plf (stmt, STMT_NECESSARY, false);
-	  mark_stmt_if_obviously_necessary (stmt, aggressive);
+	  mark_stmt_if_obviously_necessary (stmt, &necessary_decls, aggressive);
 	}
     }
 
+  /* Check all debug bind statements again in the basic blocks and find out
+     those which set an unnecessary DEBUG_EXPR_DECL to a value.  */
+  if (MAY_HAVE_DEBUG_BIND_STMTS)
+    FOR_EACH_BB_FN (bb, cfun)
+      for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+	{
+	  stmt = gsi_stmt (gsi);
+	  if (gimple_debug_bind_p (stmt)
+	      && gimple_debug_bind_has_value_p (stmt))
+	    {
+	      tree var = gimple_debug_bind_get_var (stmt);
+	      if (TREE_CODE (var) == DEBUG_EXPR_DECL
+		  && !bitmap_bit_p (necessary_decls, DECL_UID (var)))
+		{
+		  gimple_set_plf (stmt, STMT_NECESSARY, false);
+		  if (dump_file && (dump_flags & TDF_DETAILS))
+		    {
+		      fprintf (dump_file, "Unmarking useful stmt: ");
+		      print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
+		      fprintf (dump_file, "\n");
+		    }
+		}
+	      }
+	  }
+
   /* Pure and const functions are finite and thus have no infinite loops in
      them.  */
   flags = flags_from_decl_or_type (current_function_decl);

  parent reply	other threads:[~2023-04-26  9:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-25  9:33 Eric Botcazou
2023-04-25 15:10 ` Richard Biener
2023-04-25 15:20   ` Jakub Jelinek
2023-04-25 15:35     ` Eric Botcazou
2023-04-25 15:37       ` Jakub Jelinek
2023-04-26  9:31   ` Eric Botcazou [this message]
2023-04-26  9:48     ` 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=4477930.LvFx2qVVIh@fomalhaut \
    --to=botcazou@adacore.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=richard.guenther@gmail.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).