public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Correctly validate free registers for peep2
@ 2013-08-05  9:40 Richard Earnshaw
  2013-08-05 17:16 ` Richard Henderson
  0 siblings, 1 reply; 2+ messages in thread
From: Richard Earnshaw @ 2013-08-05  9:40 UTC (permalink / raw)
  To: gcc-patches, rth

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

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.

[-- Attachment #2: peephole.patch --]
[-- Type: text/plain, Size: 2548 bytes --]

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Correctly validate free registers for peep2
  2013-08-05  9:40 [PATCH] Correctly validate free registers for peep2 Richard Earnshaw
@ 2013-08-05 17:16 ` Richard Henderson
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Henderson @ 2013-08-05 17:16 UTC (permalink / raw)
  To: Richard Earnshaw; +Cc: gcc-patches

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~

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-08-05 17:16 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-05  9:40 [PATCH] Correctly validate free registers for peep2 Richard Earnshaw
2013-08-05 17:16 ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).