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

Michael Kruse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 10 19:43:15 PDT 2018


Meinersbur added a comment.

In https://reviews.llvm.org/D49281#1196034, @hsaito wrote:

> 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.


Different behavior of different implementations is also a serious concern for me. I have three different implementations in mind (the current loop transformations, an extension to Polly to use this metadata, and an idealized loop-transformation pass; the latter two being more powerful is one of the motivtions for this path). Given the prototypical transformations in `TransformMetadata.rst`, I think the model is applicable to other implementations as well.

> 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.

This would unfortunately break existing behavior. E.g. `llvm.loop.distribute.enable` and `llvm.loop.vectorize,enable` can both be specified in the same loop attributes. Currently, if LoopDistribution fails, the attribute `llvm.loop.vectorize.enable` will be left untouched. If we change LoopDistribution to remove it, the loop would not be vectorized anymore (assuming the heuristic does not deem it profitable).
It also does do what motivates this patch: Neither the order of transformations be specified, nor can the same transformation be applied multiple times.


Repository:
  rL LLVM

https://reviews.llvm.org/D49281





More information about the llvm-commits mailing list