[PATCH] Fix PR42509, wrong-code from the RTL alias oracle

[PATCH] Fix PR42509, wrong-code from the RTL alias oracle

[Richard Guenther]

This fixes PR42509, a bug in nonoverlapping_memrefs_p whose
conservative fallback for NULL MEM_OFFSET assumes DECLs are not offsetted
with negative offsets - which is not true for the spill slot decl.

This is the issue bootstrap on arm-linux-gnueabi runs into
(with some other special configure options), but the issue is
generic and in this special case triggered by cross-jumping
NULLing MEM_OFFSET.

The patch provides local surgery to nonoverlapping_memrefs_p
to avoid triggering the issue.  For 4.6 we want certain MEMs
(such as the not aliased spill slot references) to be special
cased all the way through the oracle.

Bootstrapped and tested on x86_64-unknown-linux-gnu, Richard
is bootstrapping on arm-linux-gnueabi.  I'll commit this
to trunk once he confirms that the issue is fixed by this
patch.

Thanks,
Richard.

2010-04-02  Richard Guenther  <rguenther@suse.de>

	PR middle-end/42509
	* alias.c (nonoverlapping_memrefs_p): For spill-slot accesses
	require a non-NULL MEM_OFFSET.

Index: gcc/alias.c
===================================================================
*** gcc/alias.c	(revision 157942)
--- gcc/alias.c	(working copy)
*************** nonoverlapping_memrefs_p (const_rtx x, c
*** 2147,2152 ****
--- 2147,2159 ----
    if (exprx == 0 || expry == 0)
      return 0;
  
+   /* For spill-slot accesses make sure we have valid offsets.  */
+   if ((exprx == get_spill_slot_decl (false)
+        && ! MEM_OFFSET (x))
+       || (expry == get_spill_slot_decl (false)
+ 	  && ! MEM_OFFSET (y)))
+     return 0;
+ 
    /* If both are field references, we may be able to determine something.  */
    if (TREE_CODE (exprx) == COMPONENT_REF
        && TREE_CODE (expry) == COMPONENT_REF

Re: [PATCH] Fix PR42509, wrong-code from the RTL alias oracle

[Richard Earnshaw]

On Fri, 2010-04-02 at 19:06 +0200, Richard Guenther wrote:

> Bootstrapped and tested on x86_64-unknown-linux-gnu, Richard
> is bootstrapping on arm-linux-gnueabi.  I'll commit this
> to trunk once he confirms that the issue is fixed by this
> patch.
> 
> Thanks,
> Richard.
> 
> 2010-04-02  Richard Guenther  <rguenther@suse.de>
> 
> 	PR middle-end/42509
> 	* alias.c (nonoverlapping_memrefs_p): For spill-slot accesses
> 	require a non-NULL MEM_OFFSET.

Bootstrap now successful.

Full tests will probably take at least another 12 hours, but I'm pretty
sure this is fixed now.

R.

Re: [PATCH] Fix PR42509, wrong-code from the RTL alias oracle

[Matthias Klose]

On 03.04.2010 00:15, Richard Earnshaw wrote:
> On Fri, 2010-04-02 at 19:06 +0200, Richard Guenther wrote:
>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu, Richard
>> is bootstrapping on arm-linux-gnueabi.  I'll commit this
>> to trunk once he confirms that the issue is fixed by this
>> patch.
>>
>> Thanks,
>> Richard.
>>
>> 2010-04-02  Richard Guenther<rguenther@suse.de>
>>
>> 	PR middle-end/42509
>> 	* alias.c (nonoverlapping_memrefs_p): For spill-slot accesses
>> 	require a non-NULL MEM_OFFSET.
>
> Bootstrap now successful.
>
> Full tests will probably take at least another 12 hours, but I'm pretty
> sure this is fixed now.

same here. no test results before Sunday.

   Matthias

Re: [PATCH] Fix PR42509, wrong-code from the RTL alias oracle

[Richard Guenther]

On Sat, 3 Apr 2010, Matthias Klose wrote:

> On 03.04.2010 00:15, Richard Earnshaw wrote:
> > On Fri, 2010-04-02 at 19:06 +0200, Richard Guenther wrote:
> > 
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu, Richard
> > > is bootstrapping on arm-linux-gnueabi.  I'll commit this
> > > to trunk once he confirms that the issue is fixed by this
> > > patch.
> > > 
> > > Thanks,
> > > Richard.
> > > 
> > > 2010-04-02  Richard Guenther<rguenther@suse.de>
> > > 
> > > 	PR middle-end/42509
> > > 	* alias.c (nonoverlapping_memrefs_p): For spill-slot accesses
> > > 	require a non-NULL MEM_OFFSET.
> > 
> > Bootstrap now successful.
> > 
> > Full tests will probably take at least another 12 hours, but I'm pretty
> > sure this is fixed now.
> 
> same here. no test results before Sunday.

Thanks you both for testing.  I have commited the patch so automatic
testers can pick it up until tuesday.

Thanks,
Richard.

Re: [PATCH] Fix PR42509, wrong-code from the RTL alias oracle

[Matthias Klose]

On 03.04.2010 19:13, Richard Guenther wrote:
> On Sat, 3 Apr 2010, Matthias Klose wrote:
>
>> On 03.04.2010 00:15, Richard Earnshaw wrote:
>>> On Fri, 2010-04-02 at 19:06 +0200, Richard Guenther wrote:
>>>
>>>> Bootstrapped and tested on x86_64-unknown-linux-gnu, Richard
>>>> is bootstrapping on arm-linux-gnueabi.  I'll commit this
>>>> to trunk once he confirms that the issue is fixed by this
>>>> patch.
>>>>
>>>> Thanks,
>>>> Richard.
>>>>
>>>> 2010-04-02  Richard Guenther<rguenther@suse.de>
>>>>
>>>> 	PR middle-end/42509
>>>> 	* alias.c (nonoverlapping_memrefs_p): For spill-slot accesses
>>>> 	require a non-NULL MEM_OFFSET.
>>>
>>> Bootstrap now successful.
>>>
>>> Full tests will probably take at least another 12 hours, but I'm pretty
>>> sure this is fixed now.
>>
>> same here. no test results before Sunday.
>
> Thanks you both for testing.  I have commited the patch so automatic
> testers can pick it up until tuesday.
>
> Thanks,
> Richard.

test results at
http://gcc.gnu.org/ml/gcc-testresults/2010-04/msg00363.html

Re: [PATCH] Fix PR42509, wrong-code from the RTL alias oracle

[Ulrich Weigand]

Richard Guenther wrote:

> 	PR middle-end/42509
> 	* alias.c (nonoverlapping_memrefs_p): For spill-slot accesses
> 	require a non-NULL MEM_OFFSET.

What are your thoughts on backporting this to the 4.4 branch?

It seems that this problem never existed in 4.3 and earlier because
it was introduced by IRA and the spill_slot_decl logic; and the bug
is fixed in 4.5 and later.  However, it exists as latent bug in 4.4.

Now, I was unable to construct a problem to deliberately trigger
the issue (you need just the right interaction between reload,
alias, and cross-jumping ...).  However, it seems to have occured
in an out-of-tree port:
http://gcc.gnu.org/ml/gcc/2010-04/msg00243.html

Since the fix looks to me as if it couldn't really have any adverse
effects, and it fixes a potentially hard to track down latent wrong
code generation bug, it might be good to backport ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Fix PR42509, wrong-code from the RTL alias oracle

[Richard Guenther]

On Tue, 27 Jul 2010, Ulrich Weigand wrote:

> Richard Guenther wrote:
> 
> > 	PR middle-end/42509
> > 	* alias.c (nonoverlapping_memrefs_p): For spill-slot accesses
> > 	require a non-NULL MEM_OFFSET.
> 
> What are your thoughts on backporting this to the 4.4 branch?
> 
> It seems that this problem never existed in 4.3 and earlier because
> it was introduced by IRA and the spill_slot_decl logic; and the bug
> is fixed in 4.5 and later.  However, it exists as latent bug in 4.4.
> 
> Now, I was unable to construct a problem to deliberately trigger
> the issue (you need just the right interaction between reload,
> alias, and cross-jumping ...).  However, it seems to have occured
> in an out-of-tree port:
> http://gcc.gnu.org/ml/gcc/2010-04/msg00243.html
> 
> Since the fix looks to me as if it couldn't really have any adverse
> effects, and it fixes a potentially hard to track down latent wrong
> code generation bug, it might be good to backport ...

If you want to do it and do the testing a backport is fine with me.

Richard.

Re: [PATCH] Fix PR42509, wrong-code from the RTL alias oracle

[Ulrich Weigand]

Richard Guenther wrote:
> On Tue, 27 Jul 2010, Ulrich Weigand wrote:
> > Richard Guenther wrote:
> > 
> > > 	PR middle-end/42509
> > > 	* alias.c (nonoverlapping_memrefs_p): For spill-slot accesses
> > > 	require a non-NULL MEM_OFFSET.
> > 
> > What are your thoughts on backporting this to the 4.4 branch?
> 
> If you want to do it and do the testing a backport is fine with me.

OK, I'll do that.  Thanks!

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] Fix PR42509, wrong-code from the RTL alias oracle

[Ulrich Weigand]

Richard Guenther wrote:
> On Tue, 27 Jul 2010, Ulrich Weigand wrote:
> 
> > Richard Guenther wrote:
> > 
> > > 	PR middle-end/42509
> > > 	* alias.c (nonoverlapping_memrefs_p): For spill-slot accesses
> > > 	require a non-NULL MEM_OFFSET.
> > 
> > What are your thoughts on backporting this to the 4.4 branch?
>
> If you want to do it and do the testing a backport is fine with me.

I've checked this in now.  Tested on powerpc-linux and s390x-linux.

Thanks,
Ulrich

ChangeLog:

	Backport from mainline:
	2010-04-03  Richard Guenther  <rguenther@suse.de>

	PR middle-end/42509
	* alias.c (nonoverlapping_memrefs_p): For spill-slot accesses
	require a non-NULL MEM_OFFSET.

Index: gcc/alias.c
===================================================================
*** gcc/alias.c	(revision 157953)
--- gcc/alias.c	(revision 157954)
*************** nonoverlapping_memrefs_p (const_rtx x, c
*** 2147,2152 ****
--- 2147,2159 ----
    if (exprx == 0 || expry == 0)
      return 0;
  
+   /* For spill-slot accesses make sure we have valid offsets.  */
+   if ((exprx == get_spill_slot_decl (false)
+        && ! MEM_OFFSET (x))
+       || (expry == get_spill_slot_decl (false)
+ 	  && ! MEM_OFFSET (y)))
+     return 0;
+ 
    /* If both are field references, we may be able to determine something.  */
    if (TREE_CODE (exprx) == COMPONENT_REF
        && TREE_CODE (expry) == COMPONENT_REF


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com