[polly] record delinearization result and reuse it in polyhedral translation

[Tobias Grosser]

On 03/06/2014 18:22, Sebastian Pop wrote:
> Tobias Grosser wrote:
>> >This looks good in general, but it would be good to explain_why_  we
>> >are doing this and to provide a test case that shows that this is
>> >necessary.
> In the polyhedral translation there are two ways to delinearize together all
> array subscripts touching the same base address: either duplicate the code from
> scop detection to first gather all array references and then run the
> delinearization; or the solution that I implemented and that we discussed at one
> of the polly phone callsthat is to use the same delinearization info that we
> computed during scop detection.

Right. I am aware what this patch is doing. My comment was mainly for 
the commit message.

> I added a testcase that checks that we delinearize together all the subscripts,
> otherwise we would end on a different representation.  Please see attached an
> updated patch.  Ok to commit?

That's what I meant. LGTM.

>  From d84497278df8f41aebe2a79fff4715735ee648e8 Mon Sep 17 00:00:00 2001
> From: Sebastian Pop<spop at codeaurora.org>
> Date: Fri, 30 May 2014 17:12:18 -0500
> Subject: [PATCH] record delinearization result and reuse it in polyhedral
>   translation
>
> there are two ways to delinearize together all array subscripts touching the
> same base address: either duplicate the code from scop detection to first gather
> all array references and then run the delinearization; or as implemented in this
> patch, use the same delinearization info that we computed during scop detection.
>
> Without this patch, the testcase would correctly find the delinearization of the
> first array, and would fail on the delinearization of the second array: it is
> possible to find a good the delinearization for A[i][0][k] only in the context
               ^^^ grammar.

I think the point above is the main motivation of the patch. Putting it 
at the beginning of the commit message may make it even more understandable.

> of both array accesses.
>
> ; void foo(long n, long m, long o, double A[n][m][o]) {
> ;   for (long i = 0; i < n; i++)
> ;     for (long j = 0; j < m; j++)
> ;       for (long k = 0; k < o; k++) {
> ;         A[i+3][j-4][k+7] = 1.0;
> ;         A[i][0][k] = 2.0;
> ;       }
> ; }
>
> ; CHECK: [n, m, o] -> { Stmt_for_body6[i0, i1, i2] -> MemRef_A[3 + i0, -4 + i1, 7 + i2] };
> ; CHECK: [n, m, o] -> { Stmt_for_body6[i0, i1, i2] -> MemRef_A[i0, 0, i2] };
>
> Here is the output of FileCheck on the testcase without this patch:
>
> ; CHECK: [n, m, o] -> { Stmt_for_body6[i0, i1, i2] -> MemRef_A[i0, 0, i2] };
>           ^
> <stdin>:26:2: note: possible intended match here
>   [n, m, o] -> { Stmt_for_body6[i0, i1, i2] -> MemRef_A[o0] };

This was exactly the example I was looking for. Putting it also in the 
commit message case is a good idea.

Cheers,
Tobias