This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fill more delay slots in conditional returns


> Ah, OK, yea, it makes perfect sense now.  Approved.

Thanks.

> If you wanted to get ambitious, rewriting reorg.c to compute and use
> dependency links to drive its candidate selection instead of the insane
> tracking code it uses would be a huge step forward, both in terms of
> cleanliness.  It'd also help compile-time performance; we do a lot of
> work to track resources that would be totally unnecessary.

Yes, I agree that it's quite convoluted but, as Steven already said, rewriting 
it to use a more modern framework is hard because of SEQUENCEs and, frankly, I 
have neither the real incentive nor the time to start such an endeavor.

Instead I can raise the following couple of points that I also ran into with 
the private port I'm working on:

 1. When fill_simple_delay_slots is scanning backwards to find insns to put 
into slots, it calls update_reg_dead_note which collects REG_DEAD notes and 
puts them on the insn to be moved.  But emit_delay_sequence will later drop 
them on the floor.  I have one case where preserving such a REG_DEAD note is 
correct and makes a difference (slot filled vs empty).

 2. In resource.c, when mark_target_live_regs is doing its liveness analysis, 
it extracts the insns wrapped in USEs by reorg.c:

	  /* If this insn is a USE made by update_block, we care about the
	     underlying insn.  */
	  if (code == INSN && GET_CODE (PATTERN (insn)) == USE
	      && INSN_P (XEXP (PATTERN (insn), 0)))
	      real_insn = XEXP (PATTERN (insn), 0);

and proceeds with the liveness analysis without further ado.  Now, the story 
is different in find_dead_or_set_registers, which does:

	  if (GET_CODE (PATTERN (insn)) == USE)
	    {
	      /* If INSN is a USE made by update_block, we care about the
		 underlying insn.  Any registers set by the underlying insn
		 are live since the insn is being done somewhere else.  */
	      if (INSN_P (XEXP (PATTERN (insn), 0)))
		mark_set_resources (XEXP (PATTERN (insn), 0), res, 0,
				    MARK_SRC_DEST_CALL);

and thus pessimizes the analysis by making registers unnecessary live.  Again 
I have one case where not pessimizing is correct and makes a difference.

Experimental patch attached, clean on the C testsuite for the private port.

What do you think?

-- 
Eric Botcazou
Index: reorg.c
===================================================================
--- reorg.c	(revision 197617)
+++ reorg.c	(working copy)
@@ -549,6 +549,11 @@ emit_delay_sequence (rtx insn, rtx list,
 	      remove_note (tem, note);
 	      break;
 
+	    case REG_DEP_TRUE:
+	      /* This was a REG_DEAD note that we want to preserve.  */
+	      PUT_REG_NOTE_KIND (note, REG_DEAD);
+	      break;
+
 	    case REG_LABEL_OPERAND:
 	    case REG_LABEL_TARGET:
 	      /* Keep the label reference count up to date.  */
@@ -1806,8 +1811,10 @@ update_reg_dead_notes (rtx insn, rtx del
 
 	if (reg_referenced_p (XEXP (link, 0), PATTERN (insn)))
 	  {
-	    /* Move the REG_DEAD note from P to INSN.  */
+	    /* Move the REG_DEAD note from P to INSN but smuggle it so that
+	       emit_delay_sequence doesn't drop it on the floor.  */
 	    remove_note (p, link);
+	    PUT_REG_NOTE_KIND (link, REG_DEP_TRUE);
 	    XEXP (link, 1) = REG_NOTES (insn);
 	    REG_NOTES (insn) = link;
 	  }
Index: resource.c
===================================================================
--- resource.c	(revision 197617)
+++ resource.c	(working copy)
@@ -425,6 +425,7 @@ find_dead_or_set_registers (rtx target,
   for (insn = target; insn; insn = next)
     {
       rtx this_jump_insn = insn;
+      rtx real_insn = insn;
 
       next = NEXT_INSN (insn);
 
@@ -443,7 +444,6 @@ find_dead_or_set_registers (rtx target,
 	  AND_COMPL_HARD_REG_SET (pending_dead_regs, needed.regs);
 	  AND_COMPL_HARD_REG_SET (res->regs, pending_dead_regs);
 	  CLEAR_HARD_REG_SET (pending_dead_regs);
-
 	  continue;
 
 	case BARRIER:
@@ -454,14 +454,12 @@ find_dead_or_set_registers (rtx target,
 	  if (GET_CODE (PATTERN (insn)) == USE)
 	    {
 	      /* If INSN is a USE made by update_block, we care about the
-		 underlying insn.  Any registers set by the underlying insn
-		 are live since the insn is being done somewhere else.  */
+		 underlying insn.  */
 	      if (INSN_P (XEXP (PATTERN (insn), 0)))
-		mark_set_resources (XEXP (PATTERN (insn), 0), res, 0,
-				    MARK_SRC_DEST_CALL);
-
-	      /* All other USE insns are to be ignored.  */
-	      continue;
+		real_insn = XEXP (PATTERN (insn), 0);
+	      else
+		/* All other USE insns are to be ignored.  */
+		continue;
 	    }
 	  else if (GET_CODE (PATTERN (insn)) == CLOBBER)
 	    continue;
@@ -582,8 +580,8 @@ find_dead_or_set_registers (rtx target,
 	    }
 	}
 
-      mark_referenced_resources (insn, &needed, true);
-      mark_set_resources (insn, &set, 0, MARK_SRC_DEST_CALL);
+      mark_referenced_resources (real_insn, &needed, true);
+      mark_set_resources (real_insn, &set, 0, MARK_SRC_DEST_CALL);
 
       COPY_HARD_REG_SET (scratch, set.regs);
       AND_COMPL_HARD_REG_SET (scratch, needed.regs);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]