[PATCH] D119661: [LV] Support chained phis as incoming values for first-order recurs.

Mon Feb 14 01:41:16 PST 2022

david-arm added inline comments.

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8882
       // value from the backedge after all recipes have been created.
-      recordRecipeOf(cast<Instruction>(
-          Phi->getIncomingValueForBlock(OrigLoop->getLoopLatch())));
+      auto *Inc = cast<Instruction>(
+          Phi->getIncomingValueForBlock(OrigLoop->getLoopLatch()));
----------------
This change looks like it might potentially affect reductions too, since now we're potentially not recording the recipe for reductions. Should there be a reduction test for this too, or an assert that `Ingredient2Recipe[Inc]` is always non-null for reductions?

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8884
+          Phi->getIncomingValueForBlock(OrigLoop->getLoopLatch()));
+      if (Ingredient2Recipe[Inc] == nullptr)
+        recordRecipeOf(Inc);
----------------
nit: Should this just be `!Ingredient2Recipe[Inc]` as I thought LLVM convention wasn't to compare ==/!= with nullptr?

================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9268
       Builder.setInsertPoint(InsertBlock, std::next(PrevRecipe->getIterator()));
-
     auto *RecurSplice = cast<VPInstruction>(
----------------
nit: Whitespace change.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119661