Patchwork Correctly validate free registers for peep2

login
register
mail settings
Submitter Richard Earnshaw
Date Aug. 5, 2013, 9:40 a.m.
Message ID <51FF7312.2040803@arm.com>
Download mbox | patch
Permalink /patch/264624/
State New
Headers show

Comments

Richard Earnshaw - Aug. 5, 2013, 9:40 a.m.
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.
Richard Henderson - Aug. 5, 2013, 5:16 p.m.
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~

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);