[PATCH] D50820: [VPlan] Implement initial vector code generation support for simple outer loops.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Great to see this. I've left a few small comments after a first pass and will take a closer look in the next few days!

> We plan to add a handful C outer loop executable tests once the initial code generation support is committed. This patch is expected to be NFC for the inner loop vectorizer path. Since we are moving in the direction of supporting outer loop vectorization in LV, it may also be time to rename classes such as InnerLoopVectorizer.

Excellent, having some test cases in the test-suite to exercise the VPlan native path will be great.



================
Comment at: lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:592
+
+  for (Instruction &I : *Header) {
+    if (auto *Phi = dyn_cast<PHINode>(&I)) {
----------------
can we just use Header->phis() here? Using something like llvm::any_of/llvm::all_of might also make the code slightly simpler.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7806
+
+    // We currently do not preserve loopinfo/dominator analyses with outer loop
+    // vectorization. Until this is addressed, mark these analyses as preserved
----------------
Is it worth explicitly calling this out as FIXME/TODO? IIUC this could be done independently and I think it would be good to start preserving this sooner rather than later :)


================
Comment at: lib/Transforms/Vectorize/VPlan.cpp:241
     for (VPBlockBase *Block : RPOT) {
+      if (EnableVPlanNativePath) {
+        // The inner loop vectorization path does not represent loop preheader
----------------
nit: if (!EnableVPlanNativePath) continue;?


Repository:
  rL LLVM

https://reviews.llvm.org/D50820