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

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

[Jakub Jelinek]

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.

--- 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;
+}

	Jakub

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

[Jeff Law]

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

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

[Eric Botcazou]

> 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?

-- 
Eric Botcazou

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

[Jakub Jelinek]

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

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

[Jakub Jelinek]

On Tue, Mar 05, 2013 at 11:03:13PM +0100, Jakub Jelinek wrote:
> > 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.

Here is what I've actually committed (I've also removed the
!reload_completed &&, because noce_process_if_block is only called for
!!reload_completed (the only caller asserts it)).

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

	PR rtl-optimization/56484
	* ifcvt.c (noce_process_if_block): If else_bb is NULL, avoid extending
	lifetimes of hard registers on small register class machines.

	* gcc.c-torture/compile/pr56484.c: New test.

--- gcc/ifcvt.c.jj	2013-03-05 15:12:15.284564443 +0100
+++ gcc/ifcvt.c	2013-03-05 23:11:25.751625601 +0100
@@ -2491,6 +2491,12 @@ 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.  */
+	  || (REG_P (SET_SRC (set_b))
+	      && HARD_REGISTER_P (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 16:57:50.416961638 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr56484.c	2013-03-05 16:57:50.417961672 +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;
+}


	Jakub