[PATCH] D49281: [Unroll/UnrollAndJam/Vectorizer/Distribute] Add followup loop attributes.

[Hideki Saito via Phabricator via llvm-commits]

hsaito added a comment.

In https://reviews.llvm.org/D49281#1189737, @Meinersbur wrote:

> In https://reviews.llvm.org/D49281#1189643, @hsaito wrote:
>
> > What I'm not seeing from this RFC/patch is that, if the programmer specifies transformation behavior A -> B -> C, what happens if transformation A does not kick-in? Should we just warn that "A did not happen" and stop processing the request B and C?
>
>
> Yes. A warning will be emitted by the `-transform-warning` pass (Please see Passes.rst).


This part, I know you did.

> B and C cannot apply on a loop that does not exist.

I don't think this is explicitly written. Here's an example. Suppose A is vectorization and B is unroll. If a loop is somehow not vectorized. Unrolling can still happen to the non-vectorized loop. Whether we stop unrolling in this situation is what I'd like to see us being explicit about.

>> Also, if the programmer requests that the loop to be distribute in three ways and specify different transformations for each, what should the latter transformation do if the loop is distributed in two ways or four ways?
> 
> The current LoopDistribution pass unfortunately does not support this, by a goal is to make the user able to define what code should become their own loop. See [[ A Proposal for Loop-Transformation Pragmas | https://arxiv.org/abs/1805.03374 ]].
>  For the current LoopDistribution pass, only two categories of followup-attributes can be defined `noncyclic` and `cyclic`. The `noncyclic` category can be added to multiple loops.

Whether distribution currently supports that is a different issue. I'm sure we will be expanding the features in the futures. This composability discussion should encapsulate the baseline behaviors for enough of possible future situations ---- else we have to keep revising baseline behaviors, which is very bad.

>> If we are serious about introducing this kind of features, we should clearly define what should happen when the programmer intention cannot be satisfied well enough ---- when we should continue honoring  and when we should stop honoring. If we say "we should stop in all those circumstances", that should simplify the problem a lot. If we say "we should allow to continue on subset of those cases", we should clearly state which subset and why. If there are any prior discussions (or descriptions within this patch) along this lines, please point me to that.
> 
> Documented in `TransformMetadata.rst` line 57ff.

I only see the warning behavior there. I'd like to see us explicitly saying that any subsequent explicit transformation metadata will be ignored for the given loop ---- if that's what we'll agree on, or be explicit about something else we'll agree on in the terms that can be clearly explainable to the programmers. "Compiler will skip all remaining transformations after the first failed transform" is pretty straightforward to the programmers. If anyone is proposing other behaviors, I'd like to also see how to explain those behaviors to the programmers.


Repository:
  rL LLVM

https://reviews.llvm.org/D49281