Re: [PATCH] A new reload-rewrite pattern recognizer for GCC vectorizer.

[Cong Hou <congh at google dot com>]

Ping?


thanks,
Cong


On Wed, Apr 30, 2014 at 1:28 PM, Cong Hou <congh@google.com> wrote:
> Thank you for reminding me the omp possibility. Yes, in this case my
> pattern is incorrect, because I assume all aliases will be resolved by
> alias checks, which may not be true with omp.
>
> LOOP_VINFO_NO_DATA_DEPENDENCIES or LOOP_REQUIRES_VERSIONING_FOR_ALIAS
> may not help here because vect_pattern_recog() is called prior to
> vect_analyze_data_ref_dependences() in vect_analyze_loop_2().
>
> So can we detect if omp is used in the pattern recognizer? Like
> checking loop->force_vectorize? Is there any other case in which my
> assumption does not hold?
>
>
> thanks,
> Cong
>
>
> On Sat, Apr 26, 2014 at 12:54 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Thu, Apr 24, 2014 at 05:32:54PM -0700, Cong Hou wrote:
>>> In this patch a new reload-rewrite pattern detector is composed to
>>> handle the following pattern in the loop being vectorized:
>>>
>>>    x = *p;
>>>    ...
>>>    y = *p;
>>>
>>>    or
>>>
>>>    *p = x;
>>>    ...
>>>    y = *p;
>>>
>>> In both cases, *p is reloaded because there may exist other defs to
>>> another memref that may alias with p.  However, aliasing is eliminated
>>> with alias checks.  Then we can safely replace the last statement in
>>> above cases by y = x.
>>
>> Not safely, at least not for #pragma omp simd/#pragma simd/#pragma ivdep
>> loops if alias analysis hasn't proven there is no aliasing.
>>
>> So, IMNSHO you need to guard this with LOOP_VINFO_NO_DATA_DEPENDENCIES,
>> assuming it has been computed at that point already (otherwise you need to
>> do it elsewhere).
>>
>> Consider:
>>
>> void
>> foo (int *p, int *q)
>> {
>>   int i;
>>   #pragma omp simd safelen(16)
>>   for (i = 0; i < 128; i++)
>>     {
>>       int x = *p;
>>       *q += 8;
>>       *p = *p + x;
>>       p++;
>>       q++;
>>     }
>> }
>>
>> It is valid to call the above with completely unrelated p and q, but
>> also e.g. p == q, or q == p + 16 or p == q + 16.
>> Your patch would certainly break it e.g. for p == q.
>>
>>         Jakub