diff mbox

fix up hot/cold partitioning on ports that don't have long conditional branches

Message ID 25BE4659-21B4-4662-9AFA-211522A29F60@comcast.net
State New
Headers show

Commit Message

Mike Stump April 22, 2011, 10:01 p.m. UTC 
On Apr 22, 2011, at 3:28 AM, Eric Botcazou wrote:
>> This patch fixes up hot/cold partitioning on ports that don't have long
>> conditional branches.  I'll note that the entire file has lots of other
>> jump optimizations that are suspect.
> 
> Do you have a testcase for one of the ports in the tree?

Nope.  Found via an out of tree port with tree-prof.exp=pr34999.c.

> Note that parameters of function must be documented in the head comment.

Fixed.

> The patch contains long lines.

Didn't know we had switched over to caring that much.  Want me to fix all of gcc/*.[ch]?

> The ChangeLog entry doesn't look correct (relax_delay_slots mentioned 
> twice, no mention of the new parameter of follow_jumps, etc).

Fixed.

> How was the patch tested?

make check-gcc.

> Please generate patches with diff -p.

Done.


Ok?
2011-04-21  Mike Stump  <mikestump@comcast.net>

	* reorg.c (relax_delay_slots): Don't delete a jump that
	crosses a section boundary.  Pass insn to follow_jumps.
	(follow_jumps): Add jump parameter.  Don't replace a short
	conditional jump with a long conditional jump when the port
	doesn't have long conditional jumps.
	(fill_slots_from_thread): Pass insn to follow_jumps.

Comments

Eric Botcazou May 9, 2011, 8:59 a.m. UTC | #1 
[Sorry for the delay]

> > The patch contains long lines.
>
> Didn't know we had switched over to caring that much.  Want me to fix all
> of gcc/*.[ch]?

Nope, just not introduce new long lines.

> Ok?

Almost.  It looks like we now pass twice the same argument to follow_jumps, 
i.e. we have LABEL == JUMP_LABEL (JUMP).  So I'd just pass JUMP:


  /* Follow any unconditional jump at JUMP's target label; return the ultimate
     label reached by any such chain of jumps.  If the target label is not
     followed by a jump, return it.  If the chain loops or we can't find end,
     return it as well, since that tells the caller to avoid changing the insn.
     If the chain of jumps ever crosses a section boundary, and the port
     doesn't have long condition branches and JUMP is a conditional branch,
     return the first such label before any such crossing.  */
 
 static rtx
-follow_jumps (rtx label)
+follow_jumps (rtx jump)


+      /* If a label crosses a section boundary and we're thinking
+	 about changing a conditional jump to be a conditional jump
+	 across that boundary, don't do it if the port doesn't have
+	 long conditional branches.  We can however jump to the label
+	 just before we cross such a boundary.  */
+      if (!HAS_LONG_COND_BRANCH
+	  && find_reg_note (insn, REG_CROSSING_JUMP, NULL_RTX)
+	  && any_condjump_p (jump))
+	return value;

"If a jump crosses a..."


 	  if (target_label && next_active_insn (target_label) == next
-	      && ! condjump_in_parallel_p (insn))
+	      && ! condjump_in_parallel_p (insn)
+	      && find_reg_note (insn, REG_CROSSING_JUMP, NULL_RTX) == NULL_RTX)

Either !find_reg_note here or find_reg_note (...) != NULL_RTX above.
diff mbox

Patch

Index: reorg.c
===================================================================
--- reorg.c	(revision 1301)
+++ reorg.c	(working copy)
@@ -2501,15 +2501,18 @@  fill_simple_delay_slots (int non_jumps_p
 #endif
 }
 
-/* Follow any unconditional jump at LABEL;
-   return the ultimate label reached by any such chain of jumps.
-   Return null if the chain ultimately leads to a return instruction.
-   If LABEL is not followed by a jump, return LABEL.
-   If the chain loops or we can't find end, return LABEL,
-   since that tells caller to avoid changing the insn.  */
+/* Follow any unconditional jump at LABEL; return the ultimate label
+   reached by any such chain of jumps.  Return null if the chain
+   ultimately leads to a return instruction.  If LABEL is not followed
+   by a jump, return LABEL.  If the chain loops or we can't find end,
+   return LABEL, since that tells caller to avoid changing the insn.
+   If the chain of jumps ever crosses a section boundary, and the port
+   doesn't have long condition branches and the JUMP that we are
+   performing this analysis for was a conditional branch, return the
+   first such label before any such crossing.  */
 
 static rtx
-follow_jumps (rtx label)
+follow_jumps (rtx label, rtx jump)
 {
   rtx insn;
   rtx next;
@@ -2529,6 +2532,16 @@  follow_jumps (rtx label)
     {
       rtx tem;
 
+      /* If a label crosses a section boundary and we're thinking
+	 about changing a conditional jump to be a conditional jump
+	 across that boundary, don't do it if the port doesn't have
+	 long conditional branches.  We can however jump to the label
+	 just before we cross such a boundary.  */
+      if (!HAS_LONG_COND_BRANCH
+	  && find_reg_note (insn, REG_CROSSING_JUMP, NULL_RTX)
+	  && any_condjump_p (jump))
+	return value;
+
       /* If we have found a cycle, make the insn jump to itself.  */
       if (JUMP_LABEL (insn) == label)
 	return label;
@@ -2991,7 +3004,7 @@  fill_slots_from_thread (rtx insn, rtx co
 	  && redirect_with_delay_list_safe_p (insn,
 					      JUMP_LABEL (new_thread),
 					      delay_list))
-	new_thread = follow_jumps (JUMP_LABEL (new_thread));
+	new_thread = follow_jumps (JUMP_LABEL (new_thread), new_thread);
 
       if (new_thread == 0)
 	label = find_end_label ();
@@ -3342,12 +3355,14 @@  relax_delay_slots (rtx first)
 	  && (condjump_p (insn) || condjump_in_parallel_p (insn))
 	  && (target_label = JUMP_LABEL (insn)) != 0)
 	{
-	  target_label = skip_consecutive_labels (follow_jumps (target_label));
+	  target_label = skip_consecutive_labels (follow_jumps (target_label,
+								insn));
 	  if (target_label == 0)
 	    target_label = find_end_label ();
 
 	  if (target_label && next_active_insn (target_label) == next
-	      && ! condjump_in_parallel_p (insn))
+	      && ! condjump_in_parallel_p (insn)
+	      && find_reg_note (insn, REG_CROSSING_JUMP, NULL_RTX) == NULL_RTX)
 	    {
 	      delete_jump (insn);
 	      continue;
@@ -3492,7 +3507,8 @@  relax_delay_slots (rtx first)
 	{
 	  /* If this jump goes to another unconditional jump, thread it, but
 	     don't convert a jump into a RETURN here.  */
-	  trial = skip_consecutive_labels (follow_jumps (target_label));
+	  trial = skip_consecutive_labels (follow_jumps (target_label,
+							 delay_insn));
 	  if (trial == 0)
 	    trial = find_end_label ();