On Fri, Dec 6, 2013 at 9:02 AM, Bernd Edlinger
<bernd.edlin...@hotmail.de> wrote:
> Hi,
>
> On Thu, 5 Dec 2013 21:12:54, Jeff Law wrote:
>>
>> On 12/04/13 00:49, Bernd Edlinger wrote:
>>>
>>> Regarding the consistency of bitregion_start/end ,
>>>
>>> they should either both be zero, or
>>>
>>> bitregion_start <= bitpos && bitpos+bitsize-1 <= bitregion_end
>> Presumably to satisfy the consecutive bitfields are a single memory
>> location stuff from C++11. Thus the bitregion representation (C++11)
>> must totally encompass the original COMPONENT_REF. I can certainly see
>> how that holds immediately after the call to get_bit_range.
>>
>> When we adjust the address, we're just accounting for the bit position
>> (bitpos). ie, if bitpos was 16 and TO_RTX was (plus (reg) (32)). We
>> change TO_RTX to (plus (reg) (48)). This is ultimately meant to
>> simplify the resulting insns we generate by folding the bitpos
>> adjustment into the address computation. In effect, this results in
>> accessing a subobject rather than the full object. Presumably this is
>> kosher for C++11's memory model (I really don't know).
>>
>> *If* it is OK to access the subobject like this in C++11's memory model,
>> then is is necessary for bitregion_{start,end} to encompass the original
>> object, or should it be made consistent with the bitpos/bitsize? And
>> that seems to me to be the fundamental question. What is really the
>> purpose of the bitregion_{start,end} representation? If it must
>> represent the C++ object, then ISTM we don't want to mess with it. If
>> it doesn't, then why did we bother building this alternate
>> representation to begin with?
>>
>
> Yes, that is correct. let's assume bitpos = 16, and bitsize = 16,
> then we have bitregion_start = 16 and bitregion_end = 31.
> The to_rtx points to the start of the object.
>
> to_rtx = adjust_address (to_rtx, mode1, bitpos / BITS_PER_UNIT)
>
> makes to_rtx point to two bytes from the beginning.
> that is compensated by
>
> bitpos = 0.
>
> So now [to_rtx, bitpos, bitsize] means the same thing than before.
> What's wrong is that bitregion_start is still 16 and bitregion_end is still 31
> in that example.
> The correct values would be bitregion_start = 0 and bitregion_end = 15.
> that's what this new statement is meant to do:
>
>               bitregion_start = 0;
>               if (bitregion_end>= (unsigned HOST_WIDE_INT) bitpos)
>                 bitregion_end -= bitpos;
>
> I need the "if" because bitregion_end could be 0, and in that case
> I want bitregion_end to stay 0.

The patch is ok.

Thanks,
Richard.

>
> Thanks
> Bernd.
>
>>
>> Jeff

Reply via email to