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 <ja...@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
