diff mbox

Avoid extending lifetime of likely spilled hard regs in ifcvt before reload (PR rtl-optimization/56484)

Message ID 20130305162646.GJ12913@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek March 5, 2013, 4:26 p.m. UTC
Hi!

Without this patch, ifcvt extends lifetime of %eax hard register,
which causes reload/LRA ICE later on.  Combiner and other passes try hard
not to do that, even ifcvt has code for it if x is a hard register a few
lines below it, but in this case the hard register is SET_SRC (set_b).

With this patch we just use the pseudo (x) which has been initialized
from the hard register before the conditional.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2013-03-05  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/56484
	* ifcvt.c (noce_process_if_block): Before reload if else_bb
	is NULL, avoid extending lifetimes of hard registers in
	likely to spilled or small register classes.


	Jakub

Comments

Jeff Law March 5, 2013, 4:40 p.m. UTC | #1
On 03/05/2013 09:26 AM, Jakub Jelinek wrote:
> Hi!
>
> Without this patch, ifcvt extends lifetime of %eax hard register,
> which causes reload/LRA ICE later on.  Combiner and other passes try hard
> not to do that, even ifcvt has code for it if x is a hard register a few
> lines below it, but in this case the hard register is SET_SRC (set_b).
>
> With this patch we just use the pseudo (x) which has been initialized
> from the hard register before the conditional.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2013-03-05  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR rtl-optimization/56484
> 	* ifcvt.c (noce_process_if_block): Before reload if else_bb
> 	is NULL, avoid extending lifetimes of hard registers in
> 	likely to spilled or small register classes.
OK.
Jeff
Eric Botcazou March 5, 2013, 5:28 p.m. UTC | #2
> Without this patch, ifcvt extends lifetime of %eax hard register,
> which causes reload/LRA ICE later on.  Combiner and other passes try hard
> not to do that, even ifcvt has code for it if x is a hard register a few
> lines below it, but in this case the hard register is SET_SRC (set_b).
> 
> With this patch we just use the pseudo (x) which has been initialized
> from the hard register before the conditional.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2013-03-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR rtl-optimization/56484
> 	* ifcvt.c (noce_process_if_block): Before reload if else_bb
> 	is NULL, avoid extending lifetimes of hard registers in
> 	likely to spilled or small register classes.

ifcvt.c tests only small_register_classes_for_mode_p in the other places, so 
do you really need class_likely_spilled_p here?
Jakub Jelinek March 5, 2013, 10:03 p.m. UTC | #3
On Tue, Mar 05, 2013 at 06:28:13PM +0100, Eric Botcazou wrote:
> > Without this patch, ifcvt extends lifetime of %eax hard register,
> > which causes reload/LRA ICE later on.  Combiner and other passes try hard
> > not to do that, even ifcvt has code for it if x is a hard register a few
> > lines below it, but in this case the hard register is SET_SRC (set_b).
> > 
> > With this patch we just use the pseudo (x) which has been initialized
> > from the hard register before the conditional.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > 2013-03-05  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR rtl-optimization/56484
> > 	* ifcvt.c (noce_process_if_block): Before reload if else_bb
> > 	is NULL, avoid extending lifetimes of hard registers in
> > 	likely to spilled or small register classes.
> 
> ifcvt.c tests only small_register_classes_for_mode_p in the other places, so 
> do you really need class_likely_spilled_p here?

I guess I don't.  I've grepped for small_register_classes_for_mode_p and didn't see
anything in i386, so I figured out that it would be using a default (which
is false).  But apparently it uses hook_bool_mode_true, so it is a superset
of class_likely_spilled_p, guess I can leave that out.

	Jakub
diff mbox

Patch

--- gcc/ifcvt.c.jj	2013-01-11 09:02:48.000000000 +0100
+++ gcc/ifcvt.c	2013-03-05 12:36:19.217251997 +0100
@@ -2491,6 +2491,15 @@  noce_process_if_block (struct noce_if_in
 	  || ! noce_operand_ok (SET_SRC (set_b))
 	  || reg_overlap_mentioned_p (x, SET_SRC (set_b))
 	  || modified_between_p (SET_SRC (set_b), insn_b, jump)
+	  /* Avoid extending the lifetime of hard registers on small
+	     register class machines before reload.  */
+	  || (!reload_completed
+	      && REG_P (SET_SRC (set_b))
+	      && HARD_REGISTER_P (SET_SRC (set_b))
+	      && (targetm.class_likely_spilled_p
+		    (REGNO_REG_CLASS (REGNO (SET_SRC (set_b))))
+		  || targetm.small_register_classes_for_mode_p
+		       (GET_MODE (SET_SRC (set_b)))))
 	  /* Likewise with X.  In particular this can happen when
 	     noce_get_condition looks farther back in the instruction
 	     stream than one might expect.  */
--- gcc/testsuite/gcc.c-torture/compile/pr56484.c.jj	2013-03-05 12:42:24.972220034 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr56484.c	2013-03-05 12:41:59.000000000 +0100
@@ -0,0 +1,17 @@ 
+/* PR rtl-optimization/56484 */
+
+unsigned char b[4096];
+int bar (void);
+
+int
+foo (void)
+{
+  int a = 0;
+  while (bar ())
+    {
+      int c = bar ();
+      a = a < 0 ? a : c;
+      __builtin_memset (b, 0, sizeof b);
+    }
+  return a;
+}