On Wed, Aug 31, 2011 at 9:45 AM, Richard Guenther
<richard.guent...@gmail.com> wrote:
> On Tue, Aug 30, 2011 at 5:01 PM, Aldy Hernandez <al...@redhat.com> wrote:
>> [I'm going to respond to this piece-meal, to make sure I don't drop
>> anything. My apologies for the long thread, but I'm pretty sure it's in
>> everybody's kill file by now.]
>>
>>> + /* Be as conservative as possible on variable offsets. */
>>> + if (TREE_OPERAND (exp, 2)
>>> +&& !host_integerp (TREE_OPERAND (exp, 2), 1))
>>> + {
>>> + *byte_offset = TREE_OPERAND (exp, 2);
>>> + *maxbits = BITS_PER_UNIT;
>>> + return;
>>> + }
>>>
>>> shouldn't this be at the very beginning of the function? Because
>>> you've set *bit_offset to an offset that was _not_ calculated relative
>>
>> Sure. I assume in this case, *bit_offset would be 0, right?
>
> It would be DECL_FIELD_BIT_OFFSET of that field. Oh, and
> *byte_offset would be
>
> *byte_offset = size_binop (MULT_EXPR, TREE_OPERAND (exp, 2),
> size_int (DECL_OFFSET_ALIGN
> (field) / BITS_PER_UNIT));
>
> see expr.c:component_ref_field_offset () (which you conveniently
> could use here).
>
> Note that both TREE_OPERAND (exp, 2) and compoment_ref_field_offset
> return offsets relative to the immediate containing struct type, not
> relative to the base object like get_inner_reference does ...
> (where it is still unclear to me what we are supposed to return from this
> function ...)
>
> Thus, conservative would be using get_inner_reference here, if the
> offset is supposed to be relative to the base object.
That said, shouldn't *maxbits not at least make sure to cover the field itself?
> Richard.
>
