[PATCH] D127965: [VPlan] Move recipe implementations to separate file (NFC).

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

Nice move! Would it be reasonable to also have a matching VPlanRecipes.h, excluding VPRecipeBase?



================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:38
 #include "llvm/Support/Debug.h"
-#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/GenericDomTreeConstruction.h"
----------------
Hmm, this is not needed at all?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1139
 
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 Twine VPlanPrinter::getUID(const VPBlockBase *Block) {
----------------
Ah, this #if should stay?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1340
-
-#endif
-
----------------
Keep this #endif to end the above #if.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:1
+//===- VPlan.cpp - Vectorizer Plan ----------------------------------------===//
+//
----------------
File name above and \file description below need to be updated.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:261
+  }
+
+  case VPInstruction::CanonicalIVIncrement:
----------------
nit: while we're here, can remove empty line for consistency.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:474
+}
+
+#endif
----------------
nit: remove empty line for consistency.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:523
+
+void VPWidenPHIRecipe::print(raw_ostream &O, const Twine &Indent,
+                             VPSlotTracker &SlotTracker) const {
----------------
While we're here, and considering additional execute()'s are expected to land here soon: it makes sense to have all methods of a recipe together, including its #if'd print(), and to fuse adjacent #endif/#if pairs of recipes that have only print() defined here. But VPWidenPHIRecipe and VPWidenPointerInductionRecipe have their print() amongst other print()'s and their execute() elsewhere in the file. Would be good to keep the above rules for consistency, or have all print()'s under one #if/#endif given that they look-alike.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127965