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

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: docs/TransformMetadata.rst:17
+to determine whether or not to perform transformations, and when doing
+so, other details of howthe transformations are applied (e.g., which
+vectorization factor to select).
----------------
(Space in howthe)


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:761
 
+  if (!IgnoreUser) {
+    TransformationMode EnableMode = hasUnrollTransformation(L);
----------------
This shouldn't be needed here. Before this patch, there was a single place that checked if the loop had unroll disable pragma (HasUnrollDisablePragma at the start of tryToUnrollLoop). It seems best to keep that as-is in this patch (it's already long enough!) and remove HasUnrollDisablePragma, replacing it with the new hasUnrollTransformation & TM_Disable check. Then we won't need this IgnoreUser.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:297
+      if (InheritSomeAttrs) {
+        auto AttrName = cast<MDString>(Op->getOperand(0).get())->getString();
+        if (AttrName.startswith(InheritOptionsExceptPrefix)) {
----------------
Would this fall over if the metadata was not a string? Such as debug metadata.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49281/new/

https://reviews.llvm.org/D49281