diff mbox

Correctly validate free registers for peep2

Message ID 51FF7312.2040803@arm.com
State New
Headers show

Commit Message

Richard Earnshaw Aug. 5, 2013, 9:40 a.m. UTC 
PR 57708 is a bug where peep2_find_free_register is incorrectly
returning a register that clobbers an unsaved callee saved register.
The problem is due to the way it validates register liveness: only the
first register in the list is fully validated.  In this particular case
the problem is that the second register has not been saved in the
prologue, but all of the tests except the suitability for the mode need
to be performed for each register.

	*recog.c (peep2_find_free_register): Validate all regs in a
	multi-reg mode.

Bootstrapped on x86_64.

Ok for trunk and 4.8?  (4.7 is also affected, but I don't know of any
back-end relies on this at that point).

R.

Comments

Richard Henderson Aug. 5, 2013, 5:16 p.m. UTC | #1 
On 08/04/2013 11:40 PM, Richard Earnshaw wrote:
> 	*recog.c (peep2_find_free_register): Validate all regs in a
> 	multi-reg mode.
> 
> Bootstrapped on x86_64.
> 
> Ok for trunk and 4.8?  (4.7 is also affected, but I don't know of any
> back-end relies on this at that point).

Ok.


r~
diff mbox

Patch

--- recog.c	(revision 201440)
+++ recog.c	(local)
@@ -3124,32 +3125,53 @@  peep2_find_free_register (int from, int 
       regno = raw_regno;
 #endif
 
-      /* Don't allocate fixed registers.  */
-      if (fixed_regs[regno])
-	continue;
-      /* Don't allocate global registers.  */
-      if (global_regs[regno])
-	continue;
-      /* Make sure the register is of the right class.  */
-      if (! TEST_HARD_REG_BIT (reg_class_contents[cl], regno))
-	continue;
-      /* And can support the mode we need.  */
+      /* Can it support the mode we need?  */
       if (! HARD_REGNO_MODE_OK (regno, mode))
 	continue;
-      /* And that we don't create an extra save/restore.  */
-      if (! call_used_regs[regno] && ! df_regs_ever_live_p (regno))
-	continue;
-      if (! targetm.hard_regno_scratch_ok (regno))
-	continue;
-
-      /* And we don't clobber traceback for noreturn functions.  */
-      if ((regno == FRAME_POINTER_REGNUM || regno == HARD_FRAME_POINTER_REGNUM)
-	  && (! reload_completed || frame_pointer_needed))
-	continue;
 
       success = 1;
-      for (j = hard_regno_nregs[regno][mode] - 1; j >= 0; j--)
+      for (j = 0; success && j < hard_regno_nregs[regno][mode]; j++)
 	{
+	  /* Don't allocate fixed registers.  */
+	  if (fixed_regs[regno + j])
+	    {
+	      success = 0;
+	      break;
+	    }
+	  /* Don't allocate global registers.  */
+	  if (global_regs[regno + j])
+	    {
+	      success = 0;
+	      break;
+	    }
+	  /* Make sure the register is of the right class.  */
+	  if (! TEST_HARD_REG_BIT (reg_class_contents[cl], regno + j))
+	    {
+	      success = 0;
+	      break;
+	    }
+	  /* And that we don't create an extra save/restore.  */
+	  if (! call_used_regs[regno + j] && ! df_regs_ever_live_p (regno + j))
+	    {
+	      success = 0;
+	      break;
+	    }
+
+	  if (! targetm.hard_regno_scratch_ok (regno + j))
+	    {
+	      success = 0;
+	      break;
+	    }
+
+	  /* And we don't clobber traceback for noreturn functions.  */
+	  if ((regno + j == FRAME_POINTER_REGNUM
+	       || regno + j == HARD_FRAME_POINTER_REGNUM)
+	      && (! reload_completed || frame_pointer_needed))
+	    {
+	      success = 0;
+	      break;
+	    }
+
 	  if (TEST_HARD_REG_BIT (*reg_set, regno + j)
 	      || TEST_HARD_REG_BIT (live, regno + j))
 	    {
@@ -3157,6 +3179,7 @@  peep2_find_free_register (int from, int 
 	      break;
 	    }
 	}
+
       if (success)
 	{
 	  add_to_hard_reg_set (reg_set, mode, regno);