Patchwork Fix PR55882

login
register
mail settings
Submitter Richard Guenther
Date Jan. 10, 2013, 1:53 p.m.
Message ID <alpine.LNX.2.00.1301101450350.6889@zhemvz.fhfr.qr>
Download mbox | patch
Permalink /patch/211007/
State New
Headers show

Comments

Richard Guenther - Jan. 10, 2013, 1:53 p.m.
On Thu, 10 Jan 2013, Michael Matz wrote:

> Hi,
> 
> On Thu, 10 Jan 2013, Eric Botcazou wrote:
> 
> > > Going over this what's strange as well is that we adjust MEM_SIZE
> > > with bitpos, too.  At least when the MEM has non-BLKmode this
> > > means that MEMs mode and MEM_SIZE is inconsistent?  Or how do
> > > a MEMs mode and MEM_SIZE relate?
> > 
> > Yes, the MEM attributes are incomplete/inconsistent between the call to 
> > set_mem_attributes_minus_bitpos and the subsequent adjustment by bitpos.
> 
> The problem with this intermediate inconsistency is that ...
> 
> > That's why only the final result should matter and need be correct.
> 
> ... this can't be ensured anymore.  In some cases the inconsistency 
> between the mem attributes and what XEXP(MEM, 0) represents can't be 
> repaired by the later bitpos adjustments, or better it can be only be
> repaired by falling back to the conservative side for e.g. the alignment, 
> because we don't store enough information in the mem attributes to recover 
> what was lost.
> 
> When briefly discussing this yesterday I suggested (without having the 
> code in front of me) that it be best to simply set the mem attributes only 
> at the very end, after having computed to final MEM address including all 
> adjustments, instead of generating wrong mem attributes initially and 
> hoping for the adjustments to make it come out right at the end.

Btw, if the expand_assignment code is really special we should move
minus-bitpos handling to its sole caller like with the following
(_minus_bitpos variant to be unified with set_mem_attributes and
set_mem_attributes_1 to be introduced that just computes new mem
attributes so that expand_assignment can modify them in a more
cheap way and call set_mem_attrs itself).

Richard.

Patch

Index: gcc/expr.c
===================================================================
--- gcc/expr.c	(revision 195083)
+++ gcc/expr.c	(working copy)
@@ -4856,7 +4856,16 @@  expand_assignment (tree to, tree from, b
 		 DECL_RTX of the parent struct.  Don't munge it.  */
 	      to_rtx = shallow_copy_rtx (to_rtx);
 
-	      set_mem_attributes_minus_bitpos (to_rtx, to, 0, bitpos);
+	      set_mem_attributes_minus_bitpos (to_rtx, to, 0, 0);
+	      if (bitpos / BITS_PER_UNIT != 0)
+		{
+		  if (MEM_OFFSET_KNOWN_P (to_rtx))
+		    set_mem_offset (to_rtx,
+				    MEM_OFFSET (to_rtx) - bitpos / BITS_PER_UNIT);
+		  if (MEM_SIZE_KNOWN_P (to_rtx))
+		    set_mem_size (to_rtx,
+				  MEM_SIZE (to_rtx) + bitpos / BITS_PER_UNIT);
+		}
 
 	      /* Deal with volatile and readonly fields.  The former is only
 		 done for MEM.  Also set MEM_KEEP_ALIAS_SET_P if needed.  */
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	(revision 195083)
+++ gcc/emit-rtl.c	(working copy)
@@ -1566,9 +1566,8 @@  get_mem_align_offset (rtx mem, unsigned
 
 void
 set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
-				 HOST_WIDE_INT bitpos)
+				 HOST_WIDE_INT)
 {
-  HOST_WIDE_INT apply_bitpos = 0;
   tree type;
   struct mem_attrs attrs, *defattrs, *refattrs;
   addr_space_t as;
@@ -1736,7 +1735,6 @@  set_mem_attributes_minus_bitpos (rtx ref
 	  attrs.expr = t;
 	  attrs.offset_known_p = true;
 	  attrs.offset = 0;
-	  apply_bitpos = bitpos;
 	  new_size = DECL_SIZE_UNIT (t);
 	  attrs.align = DECL_ALIGN (t);
 	  align_computed = true;
@@ -1758,7 +1756,6 @@  set_mem_attributes_minus_bitpos (rtx ref
 	  attrs.expr = t;
 	  attrs.offset_known_p = true;
 	  attrs.offset = 0;
-	  apply_bitpos = bitpos;
 	  if (DECL_BIT_FIELD (TREE_OPERAND (t, 1)))
 	    new_size = DECL_SIZE_UNIT (TREE_OPERAND (t, 1));
 	}
@@ -1809,7 +1806,6 @@  set_mem_attributes_minus_bitpos (rtx ref
 		  align_computed = true;
 		  attrs.offset_known_p = true;
 		  attrs.offset = ioff;
-		  apply_bitpos = bitpos;
 		}
 	    }
 	  else if (TREE_CODE (t2) == COMPONENT_REF)
@@ -1820,7 +1816,6 @@  set_mem_attributes_minus_bitpos (rtx ref
 		{
 		  attrs.offset_known_p = true;
 		  attrs.offset = tree_low_cst (off_tree, 1);
-		  apply_bitpos = bitpos;
 		}
 	      /* ??? Any reason the field size would be different than
 		 the size we got from the type?  */
@@ -1834,7 +1829,6 @@  set_mem_attributes_minus_bitpos (rtx ref
 	  attrs.expr = t;
 	  attrs.offset_known_p = true;
 	  attrs.offset = 0;
-	  apply_bitpos = bitpos;
 	}
 
       if (!align_computed)
@@ -1852,17 +1846,6 @@  set_mem_attributes_minus_bitpos (rtx ref
       attrs.size = tree_low_cst (new_size, 1);
     }
 
-  /* If we modified OFFSET based on T, then subtract the outstanding
-     bit position offset.  Similarly, increase the size of the accessed
-     object to contain the negative offset.  */
-  if (apply_bitpos)
-    {
-      gcc_assert (attrs.offset_known_p);
-      attrs.offset -= apply_bitpos / BITS_PER_UNIT;
-      if (attrs.size_known_p)
-	attrs.size += apply_bitpos / BITS_PER_UNIT;
-    }
-
   /* Now set the attributes we computed above.  */
   attrs.addrspace = as;
   set_mem_attrs (ref, &attrs);