[PATCH] D75980: [LV] Generate RT checks up-front and remove them if required. (WIP)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

I approve of the general direction of this patch series :)
Some high-level notes. I'm not that familiar with the code in question,
so it will be best for someone else to review, too.



================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:2649
   // Sort so that earlier instructions do not dominate later instructions.
-  sort(InsertedInstructions,
-       [this](Instruction *A, Instruction *B) { return DT.dominates(B, A); });
+  stable_sort(InsertedInstructions, [this](Instruction *A, Instruction *B) {
+    return DT.dominates(B, A);
----------------
Should this go into a separate patch? Is this a preexisting problem?


================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:2654-2659
+    for (Value *U : I->users()) {
+      assert(InsertedSet.contains(cast<Instruction>(U)));
+    }
     assert(all_of(I->users(), [&InsertedSet](Value *U) {
       return InsertedSet.contains(cast<Instruction>(U));
     }));
----------------
Are these two assertions not identical?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:799
+  /// Structure to hold information about generated runtime checks, responsible
+  /// for cleaning the checks, if they turn out unprofitable.
+  GeneratedRTChecks &Check;
----------------
s/they/vectorization/


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1623
+
+    if (MemRuntimeCheck && !isa<Constant>(MemRuntimeCheck))
+      MemRuntimeCheck->replaceAllUsesWith(
----------------
I'm guessing it can't be `i1 true` ?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2883-2886
+  auto *PHTerm = Pred->getTerminator();
+  for (unsigned i = 0; i < PHTerm->getNumSuccessors(); i++)
+    if (PHTerm->getSuccessor(i) == LoopVectorPreHeader)
+      PHTerm->setSuccessor(i, SCEVCheckBlock);
----------------



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2936-2938
+  for (unsigned i = 0; i < PHTerm->getNumSuccessors(); i++)
+    if (PHTerm->getSuccessor(i) == LoopVectorPreHeader)
+      PHTerm->setSuccessor(i, Check.TmpBlock);
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75980