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

Richard Sandiford rdsandiford@googlemail.com
Thu Nov 29 10:31:00 GMT 2012


Eric Botcazou <ebotcazou@adacore.com> writes:
>> This is because the structure we are given is:
>> 
>>     (mem/v/j:SI (reg/v/f:SI 110 [ t ]) [3 t_2(D)->a+0 S1 A32])
>> 
>> i.e. a 1-byte SImode reference.  The strange size comes from the
>> set_mem_attributes_minus_bitpos code that was mentioned earlier
>> in the series, where we set the size based on the type even if
>> it doesn't match the mode.
>
> I think that's wrong, we should have S4 and drop the MEM_EXPR as we would do 
> it with adjust_bitfield_address.  In other words, if the size computed from 
> the tree is smaller than the mode size, we don't use it and clear MEM_EXPR.

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.

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.

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

Richard



More information about the Gcc-patches mailing list