On Mon, Nov 25, 2013 at 1:07 PM, Bernd Edlinger
<bernd.edlin...@hotmail.de> wrote:
> Hello,
>
> I had forgotten to run the Ada test suite when I submitted the previous
> version of this patch.
> And indeed there were some Ada test cases failing because in Ada packed
> structures are
> like bit fields, but without the DECL_BIT_FIELD_TYPE attribute.
I think they may have DECL_BIT_FIELD set though, not sure.
> Please find attached the updated version of this patch.
>
> Boot-strapped and regression-tested on x86_64-linux-gnu.
> Ok for trunk?
So you mimic what Eric added in get_bit_range? Btw, I'm not sure
the "conservative" way of failing get_bit_range with not limiting the
access at all is good.
That is, we may want to do
+ /* The C++ memory model naturally applies to byte-aligned fields.
+ However, if we do not have a DECL_BIT_FIELD_TYPE but BITPOS or
+ BITSIZE are not byte-aligned, there is no need to limit the range
+ we can access. This can occur with packed structures in Ada. */
+ if (bitregion_start == 0 && bitregion_end == 0
+ && bitsize > 0
+ && bitsize % BITS_PER_UNIT == 0
+ && bitpos % BITS_PER_UNIT == 0)
+ {
+ bitregion_start = bitpos;
+ bitregion_end = bitpos + bitsize - 1;
+ }
thus not else if but also apply it when get_bit_range "failed" (as it may
fail for other reasons). A better fallback would be to track down
the outermost byte-aligned handled-component and limit the access
to that (though I guess Ada doesn't care at all about the C++ memory
model and only Ada has bit-aligned aggregates).
That said, the patch looks ok as-is to me, let's see if we can clean
things up for the next stage1.
Thanks,
Richard.
> Bernd.
>
>> On Mon, 18 Nov 2013 11:37:05, Bernd Edlinger wrote:
>>
>> Hi,
>>
>> On Fri, 15 Nov 2013 13:30:51, Richard Biener wrote:
>>>> That looks like always pretending it is a bit field.
>>>> But it is not a bit field, and bitregion_start=bitregion_end=0
>>>> means it is an ordinary value.
>>>
>>> I don't think it is supposed to mean that. It's supposed to mean
>>> "the access is unconstrained".
>>>
>>
>> Ok, agreed, I did not regard that as a feature.
>> And apparently only the code path in expand_assigment
>> really has a problem with it.
>>
>> So here my second attempt at fixing this.
>>
>> Boot-strapped and regression-tested on x86_64-linux-gnu.
>>
>> OK for trunk?
>>
>>
>> Thanks
>> Bernd.
