[PATCH] D41302: [LoopSimplify] Preserve Post Dom Trees across Loop Simplify

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

I assume this depends on https://reviews.llvm.org/D41299?



================
Comment at: lib/Target/AMDGPU/SIAnnotateControlFlow.cpp:375
 
-    BB = SplitBlockPredecessors(BB, Preds, "endcf.split", DT, LI, false);
+    BB = SplitBlockPredecessors(BB, Preds, "endcf.split", DT, nullptr, LI,
+                                false);
----------------
Why do we try to preserve the DominatorTree here, but not the PostDominatorTree? Here and in other files below


================
Comment at: lib/Transforms/Utils/BasicBlockUtils.cpp:324
     DT->splitBlock(NewBB);
+  if (PDT)
+    PDT->splitBlock(NewBB);
----------------
Maybe add a comment here like for updating the dominator tree?


================
Comment at: lib/Transforms/Utils/BasicBlockUtils.cpp:508
   bool HasLoopExit = false;
-  UpdateAnalysisInformation(BB, NewBB, Preds, DT, LI, PreserveLCSSA,
+  UpdateAnalysisInformation(BB, NewBB, Preds, DT, PDT, LI, PreserveLCSSA,
                             HasLoopExit);
----------------
Update comment to include PostDominator


================
Comment at: lib/Transforms/Utils/BasicBlockUtils.cpp:581
 
     // Update DominatorTree, LoopInfo, and LCCSA analysis information.
     HasLoopExit = false;
----------------
Update comment to include postdominator


================
Comment at: lib/Transforms/Utils/LoopSimplify.cpp:460
   DT->splitBlock(BEBlock);
+  if (PDT)
+    PDT->splitBlock(BEBlock);
----------------
Update postdominator comment? PDT is optional here but DT is not. I assume that has to do with some instances passing nullptr as above. Is there any reason we cannot make PDT required?`I am not saying we need to do all this in a single commit, but we should keep that in mind.


================
Comment at: lib/Transforms/Utils/LoopSimplify.cpp:825
 #endif
+
   return Changed;
----------------
unrelated whitespace change.


https://reviews.llvm.org/D41302