Patchwork [PR44610] don't delegitimize MEM from base without offset

login
register
mail settings
Submitter Alexandre Oliva
Date June 24, 2010, 3:47 a.m.
Message ID <orhbktccs6.fsf@livre.localdomain>
Download mbox | patch
Permalink /patch/56741/
State New
Headers show

Comments

Alexandre Oliva - June 24, 2010, 3:47 a.m.
Interesting bug...  All words of a multi-word variable ended up mapped
to the same VALUE during cselib in var-tracking, and then their location
was set to the same REG, that held only the first word.

Problem was that we were initializing the variable within a loop, from
an array element that lived in memory and varied per iteration of the
loop.  This means the MEM_OFFSETs couldn't be set: they weren't
constant.

Turns out that some recently-added code that attempted to simplify MEMs
to something more palatable than complex expressions involving frame,
stack or GOT pointers, mistook the absence of an offset for a zero
offset.  So, because of this delegitimization, each of the words loaded
into part of the variable was regarded by cselib as a MEM referencing
the first word of the array.  Oops.

This patch prevents the delegitimization when the offset is unknown, so
that we don't get bogus equivalences.

Regstrapped on x86_64-linux-gnu, i686-linux-gnu and ia64-linux-gnu.

Ok for trunk?
Eric Botcazou - June 25, 2010, 7:27 a.m.
> This patch prevents the delegitimization when the offset is unknown, so
> that we don't get bogus equivalences.
>
> Regstrapped on x86_64-linux-gnu, i686-linux-gnu and ia64-linux-gnu.
>
> Ok for trunk?

OK for trunk and 4.5 branch.
H.J. Lu - Oct. 3, 2010, 5:53 a.m.
On Wed, Jun 23, 2010 at 8:47 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> Interesting bug...  All words of a multi-word variable ended up mapped
> to the same VALUE during cselib in var-tracking, and then their location
> was set to the same REG, that held only the first word.
>
> Problem was that we were initializing the variable within a loop, from
> an array element that lived in memory and varied per iteration of the
> loop.  This means the MEM_OFFSETs couldn't be set: they weren't
> constant.
>
> Turns out that some recently-added code that attempted to simplify MEMs
> to something more palatable than complex expressions involving frame,
> stack or GOT pointers, mistook the absence of an offset for a zero
> offset.  So, because of this delegitimization, each of the words loaded
> into part of the variable was regarded by cselib as a MEM referencing
> the first word of the array.  Oops.
>
> This patch prevents the delegitimization when the offset is unknown, so
> that we don't get bogus equivalences.
>
> Regstrapped on x86_64-linux-gnu, i686-linux-gnu and ia64-linux-gnu.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45870

Patch

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/44610
	* simplify-rtx.c (delegitimize_mem_from_attrs): Don't use a base
	address if the offset is unknown.

Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c.orig	2010-06-23 01:15:14.000000000 -0300
+++ gcc/simplify-rtx.c	2010-06-23 01:20:21.000000000 -0300
@@ -208,10 +208,11 @@  avoid_constant_pool_reference (rtx x)
 rtx
 delegitimize_mem_from_attrs (rtx x)
 {
+  /* MEMs without MEM_OFFSETs may have been offset, so we can't just
+     use their base addresses as equivalent.  */
   if (MEM_P (x)
       && MEM_EXPR (x)
-      && (!MEM_OFFSET (x)
-	  || GET_CODE (MEM_OFFSET (x)) == CONST_INT))
+      && MEM_OFFSET (x))
     {
       tree decl = MEM_EXPR (x);
       enum machine_mode mode = GET_MODE (x);
@@ -264,8 +265,7 @@  delegitimize_mem_from_attrs (rtx x)
 	{
 	  rtx newx;
 
-	  if (MEM_OFFSET (x))
-	    offset += INTVAL (MEM_OFFSET (x));
+	  offset += INTVAL (MEM_OFFSET (x));
 
 	  newx = DECL_RTL (decl);