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.
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);
