Re: [0/8] Add optabs alternatives for insv, extv and extzv

[Eric Botcazou <ebotcazou@adacore.com>]

> I agree that this kind of MEM is less than ideal, but I thought:
> 
> 	      set_mem_attributes_minus_bitpos (to_rtx, to, 0, bitpos);
> 
> said that the attributes of TO_RTX will to be TO _once a pending offset-and-
> narrowing operation has been applied_.  So we have:
> 
>   /* 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;
>     }
> 
> I didn't think we necessarily expected the width of the reference
> (TO_RTX) and the width of the type (TO) to match at this stage.
> That's different from adjust_bitfield_address, where the
> offset-and-narrowing operation itself is applied.

I was essentially thinking of the size adjustment just above that one: if the 
mode size is known, setting a smaller size without further ado seems awkward.

So the questionable MEM doesn't survive long?  OK, maybe...

> The difference between the width of the reference and the width
> of T is what led to:
> 
>    http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00262.html
> 
> As things stand, APPLY_BITPOS is only nonzero if we set both the
> MEM_EXPR and MEM_SIZE from T.  There are also cases (like this one)
> where we don't set the MEM_EXPR from T but do set the MEM_SIZE from T.
> The bitpos will be applied either way, so I thought MEM_SIZE should be
> the same in both cases.  That doesn't fix this problem of course, it's
> just an argument that the relationship between the width of the reference
> mode, the MEM_SIZE and the width of T seems pretty complicated with the
> current interface.

MEM_SIZE and MEM_EXPR are used alone by the aliasing machinery to disambiguate 
memory references, so they need be conservative wrt the actual memory access.

> Maybe set_mem_attributes_minus_bitpos (but not set_mem_attributes)
> should only set the MEM_EXPR and leave the MEM_SIZE unchanged?
> 
> Before submitting the patched linked above, I tried getting rid
> of set_mem_attributes_minus_bitpos and passing the tree down instead.
> Then we could set the attributes at the time of the offset-and-narrowing
> operation, where the size and offset of the final reference are known.
> That didn't seem like an easy change to make though, and became a
> bit of a distraction from the main patches.
> 
> Anyway, given the breakage that this series has already caused,
> I'd prefer not to tackle stuff like this as well.  I'd only used
> MEM_SIZE in the first attempted patch out of habit.  I think the
> revised patch more obviously matches the *_fixed_bit_field functions
> and is more generally in keeping with the existing checks.
> (It's deliberately more conservative though, only using register
> bitfields if both the bit_field_mode_iterator and strict volatile
> bitfield rules are met.)

Well, rewriting the bitfield machinery of the middle-end is a once-in-a-decade 
undertaking, so some fallouts are to be expected. :-)  That wasn't too bad in 
the end.  But I agree with the cautious approach from now on.

-- 
Eric Botcazou