[PATCH] D59832: [LoopRotation] Allow loop header duplication if vectorization is forced

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

This might be a more general problem, other passes might expect a normalized form as well, such as UnrollAndJam and LoopDistribute.

For LoopVectorizer, I am somewhat surprised. It calls simplifyLoop itself (instead of relying on LoopSimplifyPass). Could the same be done for LoopRotation?



================
Comment at: lib/Transforms/Scalar/LoopRotation.cpp:60
 
+static bool isForcedVectorize(Loop *L) {
+  Optional<const MDOperand *> Value =
----------------
Consider using `hasVectorizeTransformation` in LoopUtils.h. I added it to make determining whether a pragma is added consistent between passes.


================
Comment at: test/Transforms/LoopVectorize/AArch64/Oz-and-unforced-vectorize.ll:3-4
+
+; -Oz does not allow for loop header duplication so we can't vectorize this.
+; See Oz-and-unforced-vectorize.ll.
+
----------------
There is no `llvm.loop.vectorize.enable` metadata here, so without `-vectorize-loops` option, it would not be vectorized anyway.


================
Comment at: test/Transforms/LoopVectorize/AArch64/Oz-and-unforced-vectorize.ll:27
+  %mul3 = fmul float %0, %1, !dbg !21
+; CHECK-NOT: fmul <4 x float>
+  store float %mul3, float* %arrayidx2, align 4, !dbg !21, !tbaa !16
----------------
I don't think we need to regression-test that we do not vectorize. If LoopVectorize one day manages to vectorize unrotated loop, it's a good thing, not a regression.

Btw, since it's the same content, you could instead add another `RUN:` line to the other test.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59832