Marc Glisse <marc.gli...@inria.fr> wrote:
>On Mon, 4 Nov 2013, Richard Biener wrote:
>
>> On Mon, Nov 4, 2013 at 12:18 PM, Marc Glisse <marc.gli...@inria.fr>
>wrote:
>>> On Mon, 4 Nov 2013, Richard Biener wrote:
>>>
>>>> On Mon, Nov 4, 2013 at 11:55 AM, Richard Biener
>>>> <richard.guent...@gmail.com> wrote:
>>>>>
>>>>> On Fri, Nov 1, 2013 at 11:39 PM, Marc Glisse
><marc.gli...@inria.fr>
>>>>> wrote:
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> the issue was described in the PR and the message linked from
>there.
>>>>>> ao_ref_init_from_ptr_and_size calls get_ref_base_and_extent,
>which may
>>>>>> detect an array_ref of variable index, but
>ao_ref_init_from_ptr_and_size
>>>>>> never learns of it and uses the offset+size as if they were
>meaningful.
>>>>>
>>>>>
>>>>> Well... it certainly learns of it, but it chooses to ignore...
>>>>>
>>>>> if (TREE_CODE (ptr) == ADDR_EXPR)
>>>>> - ref->base = get_ref_base_and_extent (TREE_OPERAND (ptr, 0),
>>>>> - &ref->offset, &t1, &t2);
>>>>> + {
>>>>> + ref->base = get_addr_base_and_unit_offset_1 (TREE_OPERAND
>(ptr,
>>>>> 0),
>>>>> + &t, 0, &safe);
>>>>> + ref->offset = BITS_PER_UNIT * t;
>>>>> + }
>>>>>
>>>>> safe == (t1 != -1 && t1 == t2)
>>>>>
>>>>> note that ao_ref_init_from_ptr_and_size gets the size fed in as
>argument
>>>>> so I fail to see why it matters at all ...? That is, if you feed
>in a
>>>>> wrong
>>>>> size then it's the callers error.
>>>>
>>>>
>>>> I think one issue is that the above code uses
>get_ref_base_and_extent
>>>> on an address. It should have used get_addr_base_and_unit_offset.
>>>
>>>
>>> Isn't that what my patch does? Except that
>get_addr_base_and_unit_offset
>>> often gives up and returns NULL_TREE, whereas I believe we still
>want a base
>>> even if we have trouble determining a constant offset, so I modified
>>> get_addr_base_and_unit_offset_1 a bit.
>>>
>>> (not saying the result is pretty...)
>>
>> I don't think we want get_addr_base_and_unit_offset to return
>non-NULL
>> for a non-constant offset. But yes, inside your patch is the correct
>fix,
>> so if you drop changing get_addr_base_and_unit_offset ...
>
>That means that in a number of cases, ao_ref_init_from_ptr_and_size
>will
>produce a meaningless ao_ref (both .ref and .base are 0). I expect that
>
>will cause regressions in code quality at least. Or did you mean
>something
>else?
Well, you cannot use the size argument unchanged for the null return case. You
could fallback to get_base_address and -1 size in that case.
Richard
>get_inner_reference might be an option too (we have quite a few
>functions
>doing similar things).
