[PATCH] D86850: [LoopRotate][WIP] Check code-motion safety before hoisting

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:431
   // TODO remove this limitation.
-  if (!isControlFlowEquivalent(I, InsertPoint, DT, *PDT))
+  if (PDT && !isControlFlowEquivalent(I, InsertPoint, DT, *PDT))
     return reportInvalidCandidate(I, NotControlFlowEquivalent);
----------------
This makes it easy to misuse the interface. If someone forgets to pass PDT, we skip checking for control flow equivalence and may return "safe" when it's not the case. I think we should pass a separate boolean to make the user explicitly state whether they care about control-flow equivalence or not.


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:81
+        IsUtilMode(IsUtilMode), AA(AA) {}
+  bool processLoop(Loop *L, AAResults *AA);
 
----------------
AA is a member variable, why does it need to be passed as an argument too?


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:84
 private:
-  bool rotateLoop(Loop *L, bool SimplifiedLatch);
+  bool rotateLoop(Loop *L, bool SimplifiedLatch, AAResults *AA);
   bool simplifyLoopLatch(Loop *L);
----------------
why pass AA as argument when it's available as a member?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86850