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

[Satish K Guggilla via Phabricator via llvm-commits]

sguggill added inline comments.


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


================
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
----------------
fhahn wrote:
> 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 :)
Yes, I plan to do this independently soon afterwards. I will add a TODO.


================
Comment at: lib/Transforms/Vectorize/VPlan.cpp:241
     for (VPBlockBase *Block : RPOT) {
+      if (EnableVPlanNativePath) {
+        // The inner loop vectorization path does not represent loop preheader
----------------
fhahn wrote:
> nit: if (!EnableVPlanNativePath) continue;?
I hope I am not missing something here. We want to call execute for all blocks for non VPlan-native path and skip calling execute for outer loop's PH/exit blocks.


Repository:
  rL LLVM

https://reviews.llvm.org/D50820