[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#1194814, @Meinersbur wrote:

> In https://reviews.llvm.org/D49281#1189774, @hsaito wrote:
>
> > 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.
>
>
> I added a paragraph to `TransformMetadata.rst`. (I was assuming it was obvious from the definition: A transformation in a followup-attribute only becomes assigned to a loop by the loop transformation pass. Before that, it is not associated with any loop)


The added paragraph looks good to me on the implementation side specification. Looking forward to see the programmers (i.e., compiler users, not compiler writers) side pragma description, but that will not gate my review of this patch. There is a difference between specification forcing one behavior versus implementation choice ends up in the same behavior. I wanted the former, not the latter. With this specification, we can have another implementation choice ---- attaching all those metadata to the loop, to be updated by the successful transformation, and let failed transform drop subsequent ones. I'm not saying it's better to go that way. What I'm saying is that if, for some reason, we later choose to implement this differently, there is a specification to guide how to implement the feature correctly. Hope I don't sound too picky. I just want to provide consistent experiences to the programmers.


Repository:
  rL LLVM

https://reviews.llvm.org/D49281