Reductions [patch 2 & 3]

[Tobias Grosser]

On 16/06/2014 08:24, Tobias Grosser wrote:
> On 15/06/2014 23:45, jdoerfert at codeaurora.org wrote:
>> Comments inlined.
>
> Dito.
>
>>
>> --
>>
>> Johannes Doerfert
>> Employee of Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> hosted by The Linux Foundation
>>
>>>>>>> +static cl::opt<bool, true>
>>>>>>> +XEnableReductions("polly-reductions",
>>>>>>> +                  cl::desc("Relax the reduction dependences before
>>>> scheduling"),
>>>>>>> +                  cl::Hidden,
>>>>>>> cl::location(polly::EnableReductions),
>>>>>>> +                  cl::init(false), cl::ZeroOrMore,
>>>>>>> +cl::cat(PollyCategory)); bool polly::EnableReductions = false;
>>>>>>
>>>>>> Should we enable this by default? Do we even need an option?
>>>> Without an updated code generation I would like to keep it under a
>>>> flag.
>>>
>>> Why? This should not cause any correctness issues so enabling it gives
>>> us test coverage. Also, the schedule optimizer may already benefit from
>>> the increased freedom due to removing the reduction dependences.
>> It will affect correctness as soon as the vectorizer or parallelizer is
>> used, furthermore we need "privatization dependences" before we can
>> safely
>> use it in a sequential environment. As soon as it is safe to enable it
>> (maybe only in sequential scenarios) I will.
>
> So there is no way to make the initial patches save by default? I am
> trying to keep patches as small as possible, but if some pieces are
> needed for correctness than we should add those. Specifically, can
> we ensure that the analysis the vectorizer/parallizer use do include the
> reduction dependences as long as they do not yet perform privatization.

I just thought about this more. For parallelism checks, we need to look 
both at the reduction dependences as well as the non-reduction 
dependences. However, as our sequential transformations may have changed 
the schedule in a way that reduction dependences have been reversed 
(this is legal), we most likely need to adjust/recompute them
such that they are positive according to the new schedule. After this
we can use them in the parallelism check that does not assume privatization.

Cheers,
Tobias