[PATCH] D47683: [PM/LoopUnswitch] Teach the new unswitch to handle nontrivial unswitching of switches.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added a comment.

Thanks for comments, should all be addressed



================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:731
     BasicBlock *UnswitchedSuccBB, BasicBlock *ContinueSuccBB,
-    const SmallPtrSetImpl<BasicBlock *> &SkippedLoopAndExitBlocks,
+    const SmallDenseMap<BasicBlock *, BasicBlock *, 16> &DominatingSucc,
     ValueToValueMapTy &VMap,
----------------
asbirlea wrote:
> Could you update the above comment to clarify who DominatingSucc is?
> e.g. "It skips loop and exit blocks if their dominating successor (`DominatingSucc`) is not the block being unswitched." ?
I used different phrasing, but let me know if it didn't work.


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:1747
+    for (BasicBlock *SuccBB : UnswitchedSuccBBs) {
+      // Remove the parent as a predecessor of the
+      // unswitched successor.
----------------
asbirlea wrote:
> clang-format?
Fixed.


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:1798
   SmallVector<BasicBlock *, 16> DeadBlocks;
-  if (DeleteUnswitchedSucc) {
-    DeadBlocks.push_back(UnswitchedSuccBB);
+#if 0
+  for (auto *SuccBB : UnswitchedSuccBBs) {
----------------
hfinkel wrote:
> Dead code?
Doh! sorry i didn't delete this.


Repository:
  rL LLVM

https://reviews.llvm.org/D47683