> expand_assignment calls:
>
> if (MEM_P (to_rtx))
> {
> /* If the field is at offset zero, we could have been given
> the 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);
> ...
The MEM_KEEP_ALIAS_SET_P line seems to be redundant here (but not the
MEM_VOLATILE_P line).
> But set_mem_attributes_minus_bitpos has:
>
> /* Default values from pre-existing memory attributes if present. */
> refattrs = MEM_ATTRS (ref);
> if (refattrs)
> {
> /* ??? Can this ever happen? Calling this routine on a MEM that
> already carries memory attributes should probably be invalid. */
> attrs.expr = refattrs->expr;
> attrs.offset_known_p = refattrs->offset_known_p;
> attrs.offset = refattrs->offset;
> attrs.size_known_p = refattrs->size_known_p;
> attrs.size = refattrs->size;
> attrs.align = refattrs->align;
> }
>
> which of course applies in this case: we have a MEM for g__style.
> I would expect many calls from this site are for MEMs with an
> existing MEM_EXPR.
Indeed. Not very clear what to do though.
> Then later:
>
> /* If this is a field reference and not a bit-field, record it. */
> /* ??? There is some information that can be gleaned from bit-fields,
> such as the word offset in the structure that might be modified.
> But skip it for now. */
> else if (TREE_CODE (t) == COMPONENT_REF
> && ! DECL_BIT_FIELD (TREE_OPERAND (t, 1)))
>
> so we leave the offset and expr alone. The end result is:
>
> (mem/j/c:SI (symbol_ref:DI ("g__style") [flags 0x4] <var_decl
> 0x7ffff6e4ee40 g__style>) [0 g__style+0 S1 A64])
>
> an SImode reference to the first byte (and only the first byte) of g__style.
> Then, when we apply adjust_bitfield_address, it looks like we're moving
> past the end of the region and so we drop the MEM_EXPR.
>
> In cases where set_mem_attributes_minus_bitpos does set MEM_EXPR based
> on the new tree, it also adds the bitpos to the size. But I think we
> should do that whenever we set the size based on the new tree,
> regardless of whether we were able to record a MEM_EXPR too.
>
> That said, this code has lots of ???s in it, so I'm not exactly
> confident about this change. Thoughts?
It also seems a little bold to me. Since we now have the new processing of
MEM_EXPR for bitfields, can't we remove the ! DECL_BIT_FIELD check?
/* If this is a field reference and not a bit-field, record it. */
/* ??? There is some information that can be gleaned from bit-fields,
such as the word offset in the structure that might be modified.
But skip it for now. */
else if (TREE_CODE (t) == COMPONENT_REF
&& ! DECL_BIT_FIELD (TREE_OPERAND (t, 1)))
{
attrs.expr = t;
attrs.offset_known_p = true;
attrs.offset = 0;
apply_bitpos = bitpos;
/* ??? Any reason the field size would be different than
the size we got from the type? */
}
This would mean removing the first ??? comment. As for the second ???
comment, the answer is easy: because that's pretty much what defines a
bitfield! The size is DECL_SIZE_UNIT and not TYPE_SIZE_UNIT for them.
--
Eric Botcazou
