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