diff mbox

combine: Do not allow identical insns for I0,I1 and/or I2 (PR64268)

Message ID bdf8558f797b5baf9fa9d332c884b3d70d6077fe.1418771841.git.segher@kernel.crashing.org
State New
Headers show

Commit Message

Segher Boessenkool Dec. 16, 2014, 11:28 p.m. UTC
When looking deeper into the problem I discovered that very sometimes
try_combine is called with the same insn for I1 and I2.  This is quite
bad since try_combine does not expect that at all.  I expect this is
caused by my patches adding a LOG_LINK per register, which can mean
a pair of insns has more than one LOG_LINK between them.

This patch fixes it by making try_combine refuse identical insns early.

Bootstrapped on powerpc64-linux and powerpc-linux; the fails are gone.
Also bootstrapped on x86_64-linux.
Is this okay for mainline?  (regtest on powerpc64-linux in process).


Segher


2014-12-15  Segher Boessenkool  <segher@kernel.crashing.org>

gcc/
	PR target/64268
	* combine.c (try_combine): Immediately return if any of I0,I1,I2
	are the same insn.


---
 gcc/combine.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Segher Boessenkool Dec. 19, 2014, 3:13 p.m. UTC | #1
Hi,

Sorry for the quick ping, but this fixes bootstrap on AIX, powerpc-darwin,
and 32-bit PowerPC Linux.  Could a gentle soul please spare a minute to
review, before the holidays?  Thanks in advance!


Segher

p.s.  The regtests of course succeeded.


On Tue, Dec 16, 2014 at 03:28:16PM -0800, Segher Boessenkool wrote:
> When looking deeper into the problem I discovered that very sometimes
> try_combine is called with the same insn for I1 and I2.  This is quite
> bad since try_combine does not expect that at all.  I expect this is
> caused by my patches adding a LOG_LINK per register, which can mean
> a pair of insns has more than one LOG_LINK between them.
> 
> This patch fixes it by making try_combine refuse identical insns early.
> 
> Bootstrapped on powerpc64-linux and powerpc-linux; the fails are gone.
> Also bootstrapped on x86_64-linux.
> Is this okay for mainline?  (regtest on powerpc64-linux in process).
> 
> 
> Segher
> 
> 
> 2014-12-15  Segher Boessenkool  <segher@kernel.crashing.org>
> 
> gcc/
> 	PR target/64268
> 	* combine.c (try_combine): Immediately return if any of I0,I1,I2
> 	are the same insn.
> 
> 
> ---
>  gcc/combine.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/gcc/combine.c b/gcc/combine.c
> index de2e49f..8e5d1f7 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -2620,6 +2620,11 @@ try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
>    rtx new_other_notes;
>    int i;
>  
> +  /* Immediately return if any of I0,I1,I2 are the same insn (I3 can
> +     never be).  */
> +  if (i1 == i2 || i0 == i2 || (i0 && i0 == i1))
> +    return 0;
> +
>    /* Only try four-insn combinations when there's high likelihood of
>       success.  Look for simple insns, such as loads of constants or
>       binary operations involving a constant.  */
> -- 
> 1.8.1.4
Paolo Bonzini Dec. 19, 2014, 5:23 p.m. UTC | #2
On 19/12/2014 16:13, Segher Boessenkool wrote:
> Hi,
> 
> Sorry for the quick ping, but this fixes bootstrap on AIX, powerpc-darwin,
> and 32-bit PowerPC Linux.  Could a gentle soul please spare a minute to
> review, before the holidays?  Thanks in advance!

I cannot approve it, but the patch is pretty obvious.

Adding a gentle soul that happens to be a global reviewer.

Paolo
Richard Biener Dec. 19, 2014, 6:04 p.m. UTC | #3
On December 19, 2014 6:23:05 PM CET, Paolo Bonzini <bonzini@gnu.org> wrote:
>
>
>On 19/12/2014 16:13, Segher Boessenkool wrote:
>> Hi,
>> 
>> Sorry for the quick ping, but this fixes bootstrap on AIX,
>powerpc-darwin,
>> and 32-bit PowerPC Linux.  Could a gentle soul please spare a minute
>to
>> review, before the holidays?  Thanks in advance!
>
>I cannot approve it, but the patch is pretty obvious.
>
>Adding a gentle soul that happens to be a global reviewer.

OK.

Thanks,
Richard.

>Paolo
diff mbox

Patch

diff --git a/gcc/combine.c b/gcc/combine.c
index de2e49f..8e5d1f7 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -2620,6 +2620,11 @@  try_combine (rtx_insn *i3, rtx_insn *i2, rtx_insn *i1, rtx_insn *i0,
   rtx new_other_notes;
   int i;
 
+  /* Immediately return if any of I0,I1,I2 are the same insn (I3 can
+     never be).  */
+  if (i1 == i2 || i0 == i2 || (i0 && i0 == i1))
+    return 0;
+
   /* Only try four-insn combinations when there's high likelihood of
      success.  Look for simple insns, such as loads of constants or
      binary operations involving a constant.  */