diff mbox

IRA: Don't create new regs for debug insns (PR80429)

Message ID 20170419075647.GF18309@gate.crashing.org
State New
Headers show

Commit Message

Segher Boessenkool April 19, 2017, 7:56 a.m. UTC
On Tue, Apr 18, 2017 at 11:47:40AM +0200, Jakub Jelinek wrote:
> On Tue, Apr 18, 2017 at 09:30:19AM +0000, Segher Boessenkool wrote:
> > In split_live_ranges_for_shrink_wrap IRA also splits regs that are
> > only used in debug insns, leading to -fcompare-debug failures.
> > 
> > Bootstrapped and tested on powerpc64-linux {-m32,-m64}.  This happens
> > on at least GCC 5, so not a regression; but it is trivial and obvious,
> > is it okay for trunk anyway?
> 
> I think the first question here is if it is beneficial to replace the
> regs in DEBUG_INSNs or not.  If it is beneficial, then it of course
> has to be changed such that it never does
> newreg = ira_create_new_reg (dest);
> just because it sees DEBUG_INSN uses, but otherwise it should do what it
> does now.
> 
> So one option is e.g. to split that loop into two, one doing analysis,
> i.e. that newreg = ira_create_new_reg (dest);, but not
> validate_change (uin, DF_REF_REAL_LOC (use), newreg, true);
> and this loop would also ignore DEBUG_INSN_P.
> And the other loop, with the same content, just without that newreg = ...
> and with the validate_change, as whole guarded with if (newreg),
> replacing stuff on all insns.

Like this?  Tested as before, etc.  Thanks,


Segher


2017-04-18  Segher Boessenkool  <segher@kernel.crashing.org>

	PR rtl-optimization/80429
	* ira.c (split_live_ranges_for_shrink_wrap): Don't split regs that
	are only used in debug insns.

---
 gcc/ira.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

Comments

Jakub Jelinek April 19, 2017, 9:33 a.m. UTC | #1
On Wed, Apr 19, 2017 at 02:56:47AM -0500, Segher Boessenkool wrote:
> On Tue, Apr 18, 2017 at 11:47:40AM +0200, Jakub Jelinek wrote:
> > On Tue, Apr 18, 2017 at 09:30:19AM +0000, Segher Boessenkool wrote:
> > > In split_live_ranges_for_shrink_wrap IRA also splits regs that are
> > > only used in debug insns, leading to -fcompare-debug failures.
> > > 
> > > Bootstrapped and tested on powerpc64-linux {-m32,-m64}.  This happens
> > > on at least GCC 5, so not a regression; but it is trivial and obvious,
> > > is it okay for trunk anyway?
> > 
> > I think the first question here is if it is beneficial to replace the
> > regs in DEBUG_INSNs or not.  If it is beneficial, then it of course
> > has to be changed such that it never does
> > newreg = ira_create_new_reg (dest);
> > just because it sees DEBUG_INSN uses, but otherwise it should do what it
> > does now.
> > 
> > So one option is e.g. to split that loop into two, one doing analysis,
> > i.e. that newreg = ira_create_new_reg (dest);, but not
> > validate_change (uin, DF_REF_REAL_LOC (use), newreg, true);
> > and this loop would also ignore DEBUG_INSN_P.
> > And the other loop, with the same content, just without that newreg = ...
> > and with the validate_change, as whole guarded with if (newreg),
> > replacing stuff on all insns.
> 
> Like this?  Tested as before, etc.  Thanks,
> 
> 
> Segher
> 
> 
> 2017-04-18  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> 	PR rtl-optimization/80429
> 	* ira.c (split_live_ranges_for_shrink_wrap): Don't split regs that
> 	are only used in debug insns.

Ok, thanks.

	Jakub
diff mbox

Patch

diff --git a/gcc/ira.c b/gcc/ira.c
index 7079573..bfb0508 100644
--- a/gcc/ira.c
+++ b/gcc/ira.c
@@ -4992,25 +4992,40 @@  split_live_ranges_for_shrink_wrap (void)
       if (!dest || dest == pic_offset_table_rtx)
 	continue;
 
-      rtx newreg = NULL_RTX;
+      bool need_newreg = false;
       df_ref use, next;
       for (use = DF_REG_USE_CHAIN (REGNO (dest)); use; use = next)
 	{
 	  rtx_insn *uin = DF_REF_INSN (use);
 	  next = DF_REF_NEXT_REG (use);
 
+	  if (DEBUG_INSN_P (uin))
+	    continue;
+
 	  basic_block ubb = BLOCK_FOR_INSN (uin);
 	  if (ubb == call_dom
 	      || dominated_by_p (CDI_DOMINATORS, ubb, call_dom))
 	    {
-	      if (!newreg)
-		newreg = ira_create_new_reg (dest);
-	      validate_change (uin, DF_REF_REAL_LOC (use), newreg, true);
+	      need_newreg = true;
+	      break;
 	    }
 	}
 
-      if (newreg)
+      if (need_newreg)
 	{
+	  rtx newreg = ira_create_new_reg (dest);
+
+	  for (use = DF_REG_USE_CHAIN (REGNO (dest)); use; use = next)
+	    {
+	      rtx_insn *uin = DF_REF_INSN (use);
+	      next = DF_REF_NEXT_REG (use);
+
+	      basic_block ubb = BLOCK_FOR_INSN (uin);
+	      if (ubb == call_dom
+		  || dominated_by_p (CDI_DOMINATORS, ubb, call_dom))
+		validate_change (uin, DF_REF_REAL_LOC (use), newreg, true);
+	    }
+
 	  rtx_insn *new_move = gen_move_insn (newreg, dest);
 	  emit_insn_after (new_move, bb_note (call_dom));
 	  if (dump_file)