[Polly] Isl AST reduction pragmas [V2]

[Johannes Doerfert]

Hey Tobi,

I cannot come up with a test case which contains a reduction and will be
inverted.

I tried combinations of loop nest similar to your example, but no luck so
far.
Handling this case would be a nice improvement, but do we need it right now
or can we go with the simpler solution which is slightly less general?

Best regards,
  Johannes

--

Johannes Doerfert
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation


-----Original Message-----
From: Tobias Grosser [mailto:tobias at grosser.es] 
Sent: Wednesday, June 18, 2014 12:03 PM
To: Johannes Doerfert
Cc: llvm-commits at cs.uiuc.edu; 'Sebastian Pop'
Subject: Re: [Polly] Isl AST reduction pragmas [V2]

On 17/06/2014 02:52, Johannes Doerfert wrote:
> Hey Tobias,
>
>
>
> I attached a updated version of the Isl AST reduction patch.
>
> I did not check for loop reversals (one of your comments) but afaik. 
> the isl scheduler won't come up with them anyway [as it is based on 
> Uday's paper which describes it disallows them].

Unfortunately, this is one of the differences between the isl scheduler and
the Pluto scheduler. The isl scheduler does not disallow loop reversal.
Similarly, newer versions of Pluto have also been changed to allow loop
reversal:

http://www.csa.iisc.ernet.in/TR/2014/1/tr-2014-1.pdf

I just tried it myself. The code will be reversed:

void foo(float *A, long n) {

         for (long i = 0; i < n; i++)
                 A[i] = 1;
         for (long i = 0; i < n; i++)
                 A[n-i-1] += 1;


}

with the option '-polly-opt-fusion=max'. The result is:

   for (c1=1;c1<=n;c1++) {
     Stmt_for_body((c1-1));
     Stmt_for_body4((-c1+n));
   }

> We can in the future change this if necessary.

I am afraid we can not take a shortcut here.

> What do you think of this version?

I think we should find a test case where we both reverse the loops and where
we get reduction dependences and write a separate patch for the scheduler
that updates them if necessary. You should be able to use the same

>     /// @brief The type of the dependences.
> +  ///
> +  /// Reduction dependences are not included in "TYPE_ALL" 
> + dependences  /// as they are no "validity" dependences in the classical
sense.

'classical sense' is not well defined.

> +  /// Instead, they can be seen as dependences we wan't to fulfill 
> + but
                                                      want

Otherwise, I think the patch is fine. It is in fact a perfect example of the
small, targeted patches I am looking for.

Tobias