[Polly] Reduction dependences [V3]

[Johannes Doerfert]

r211369

--

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: Friday, June 20, 2014 12:54 AM
To: Johannes Doerfert
Cc: llvm-commits at cs.uiuc.edu; 'Sebastian Pop'
Subject: Re: [Polly] Reduction dependences [V3]

On 19/06/2014 23:12, Johannes Doerfert wrote:
> What do you think?
>
>> >On 19/06/2014 20:02, Johannes Doerfert wrote:
>>> > >I put it into a loop now, this copy and paste code over multiple 
>>> > >lines makes my dizzy sometimes.
>> >Great, a perfect use of range based for loops.
> I have two sets of dependences to handle, so no range loop:( It still 
> looks much better in my opinion than the repetition did.

Anyhow. ;-)

> // Reduction dependences are not included in TYPE_ALL dependences 
> because we // can ignore them during the scheduling. This is the case 
> since the order in // which the reduction statements are executed does 
> not matter. However, if // they are executed in parallel we need to 
> take additional measures (e.g., // privatization) to ensure a correct 
> result.

Works.

>>> > >+void Dependences::addPrivatizationDependences() {
>>> > >+  isl_union_map *PrivRAW, *PrivWAW, *PrivWAR, *TransClosure;
>>> > >+
>>> > >+  // The transitive closure might be over approximated but we 
>>> > >+ only use
> it to
>>> > >+  // compute the privatization dependences. Thus, 
>>> > >+ overapproximation
> will lead
>>> > >+  // "only" to more conservative privatization dependences.
>> >Can this approximation introduce dependences to statements that are 
>> >scheduled earlier?
> I don't know but I doubt it since we start only with strict forward 
> dependences.
> We need to ask Sven if we somehow can get backward dependences due to 
> overapproximation.

Thinking more about it, I am afraid with an arbitrary schedule and any kind
of approximation, we may cause backwards dependences.

>> >I am asking because I would like you to_verify_  that there is no 
>> >way that there will be wrong dependences introduced. If we are not 
>> >sure, we need to look at the dependences in the context of the 
>> >schedule and remove all negative dependences. I would like to avoid
this.
> Why do we need the schedule?

To understand if a dependency is forward.

S[i] -> S[i+1] is a backwards dependency in case the schedule is S[i] ->
[-i].

> We start with dependences strict forward in some direction and the 
> result should look the same. So we could verify it like this until we 
> know for sure there are no backward dependences created by the 
> transitive closure.
>
> I was thinking we could do it like this:
>
>   // The transitive closure might be over approximated but we only use it
to
>   // compute the privatization dependences. Thus, overapproximation will
lead
>   // "only" to more conservative privatization dependences. However, we
take
>   // precautions to ensure only forward dependences are created. This 
> will ensure
>   // that we won't create dependency cycles.
>   TransClosure = 
> isl_union_map_transitive_closure(isl_union_map_copy(RED),
> 0);
>
>   // [NEW] check for non positive dependences in the transitive 
> closure and substract them

Right. Just remove any backwards dependences according to the schedule from
the transitive closure.

>>> > >+  // FIXME: Reduction dependences prevent dce at the moment
>> >Good catch. Why do we need to fix anything here? how would we fix it?
> We dont have to fix anything now, I reformulated the comment to make 
> it more
> clear:
> // FIXME: Unary reductions will prevent dce as they will look live.
> //        We should split reduction dependences into binary and unary
> //        into RAW and WAW) to avoid this problem.
>
> Later we could split reduction dependences into reduction RAW and 
> reduction WAW and only add the RAW ones to the live set. The idea is 
> that binary reductions are live at the end while unary are not.
>
> Should I remove the comment as it is only a concern for the not yet 
> implemented unary reductions or should I leave it ?

Yes, just drop it.

As I think that it is highly unlikely that the over approximation will cause
any troubles, I am OK with you committing this patch as it is now. 
You can (and should) commit a subsequent patch that fixes the loop hole in
the overapproximation.

Tobias