Re: [RFC][patch] If-conversion of COMPONENT_REFs

[Ira Rosen <ira dot rosen at linaro dot org>]

On 30 March 2011 14:41, Richard Guenther <richard.guenther@gmail.com> wrote:
> On Wed, Mar 30, 2011 at 2:22 PM, Ira Rosen <ira.rosen@linaro.org> wrote:
>> On 30 March 2011 12:59, Richard Guenther <richard.guenther@gmail.com> wrote:
>>> On Wed, Mar 30, 2011 at 11:13 AM, Ira Rosen <ira.rosen@linaro.org> wrote:
>>>> Hi,
>>>>
>>>> With this patch a data-ref is marked as unconditionally read or
>>>> written also if its adjacent field is read or written unconditionally
>>>> in the loop.
>>>> My concern is that this is not safe enough, even though the fields
>>>> have to be non-pointers and non-aggregates, and this optimization is
>>>> applied only with -ftree-loop-if-convert-stores flag.
>>>>
>>>> Bootstrapped on powerpc64-suse-linux and tested on x86_64-suse-linux.
>>>>
>>>> OK for trunk?
>>>
>>> The restrictions do not make too much sense to me. ?For the C++
>>> memory model we can't do speculative stores at all, but for the
>>> rest I'd say just checking if the data-refs access the same object
>>> is enough, thus, instead of same_data_refs (a, b) simply check
>>> operand_equal_p (DR_BASE_ADDRESS (a), DR_BASE_ADDRESS (b), 0)
>>> or operand_equal_p (get_base_address (DR_REF (a)), get_base_address
>>> (DR_REF (b)), 0), whatever makes more sense (I always confuse what
>>> the contents of the various DR fields are).
>>
>> But what about
>>
>> int a[10], b[100], c[100];
>> for (i = 0; i < 100; i++)
>> ?{
>> ? ? if (i < 10)
>> ? ? ? t = a[i];
>> ? ?else
>> ? ? ? t = b[i];
>>
>> ? ?c[i] = t + a[1];
>> ? }
>>
>> We can't transform this to
>>
>> int a[10], b[100], c[100];
>> for (i = 0; i < 100; i++)
>> ?{
>> ? ? t1 = a[i];
>> ? ? t2 = b[i];
>> ? ? t = (i < 10) ? t1 : t2;
>> ? ? c[i] = t + a[1];
>> ? }
>>
>> since we create accesses to a[10:100], right?
>
> That's correct - we may only ever create "valid" accesses, but any
> valid access to a is ok after we see an unconditional access to a.
>
> So we probably have to restrict ourselves to DECL_P (get_base_address ())
> objects and make sure we don't access past it.
>
> Thus, I see what you were trying to do - may I suggest sth like
>
> ?ref_base_a = DR_REF (a);
> ?while (TREE_CODE (ref_base_a) == COMPONENT_REF
> ? ? ? ? ? || TREE_CODE (ref_base_a) == IMAGPART_EXPR
> ? ? ? ? ? || TREE_CODE (ref_base_a) == REALPART_EXPR)
> ? ?ref_base_a = TREE_OPERAND (ref_base_a, 0);
>
> ... same for DR_REF (b) ...
>
> ?if (operand_equal_p (ref_base_a, ref_base_b, 0))
> ? ?ok
>
> that is, strip all non-variable offset handled_component_refs and compare
> the remaining base of the two accesses. ?If they are equal we are ok.
>
> Any hole in that?

I don't see any :) I'll test your version.

Thanks,
Ira

>
> Thanks,
> Richard.
>
>> Thanks,
>> Ira
>>
>>>
>>> Thanks,
>>> Richard.
>>>
>>>> Thanks,
>>>> Ira
>>>>
>>>>
>>>> ChangeLog:
>>>>
>>>> ? ? ? ? * tree-if-conv.c (memrefs_read_or_written_unconditionally): Return true
>>>> ? ? ? ? if an adjacent field of the data-ref is accessed unconditionally.
>>>>
>>>> testsuite/ChangeLog:
>>>>
>>>> ? ? ? ? * gcc.dg/vect/if-cvt-stores-vect-ifcvt-18.c: New test.
>>>> ? ? ? ? * gcc.dg/vect/vect.exp: Run if-cvt-stores-vect* tests with
>>>> ? ? ? ? -ftree-loop-if-convert-stores.
>>>>
>>>
>>
>