[PATCH] D38558: [JumpThreading] Preserve DT and LVI across the pass.

[Brian Rzycki via Phabricator via llvm-commits]

brzycki abandoned this revision.
brzycki added a comment.

This code is superceded by https://reviews.llvm.org/D40146.



================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:692
+  if (DT && ReplacedEntryBB)
+    DT->recalculate(*(DestBB->getParent()));
 }
----------------
bmakam wrote:
> I am trying to understand why we need to recalculate DT here. Can't we just delete the edge before nuking BB instead of recalculating?
Hello @bmakam , unfortunately no. In this corner case we've removed the entry block of the function. This is treated as the root node of  the new DT data structure @kuhar implemented. If we continue as you propose the next insertEdge() or deleteEdge() returns an assert about the root node of the DT being invalid.

I had a private e-mail thread discussion with @kuhar and @dberlin last August and we decided on this compromise in this corner-case event.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:692
+  if (DT && ReplacedEntryBB)
+    DT->recalculate(*(DestBB->getParent()));
 }
----------------
bmakam wrote:
> I am trying to understand why we need to recalculate DT here. Can't we just delete the edge before nuking BB instead of recalculating?
I collected some statistics on this event: of the times we enter MergeBasicBlockIntoOnlyPred()  this case is encountered somewhere around 5% of the time:

> 4.49% of calls remove the entry block for  ninja check
> 	913 calls into MergeBasicBlockIntoOnlyPred
> 	41 entry block removals
> 
> 5.09% of calls remove the entry block for test-suite build
> 	30335 calls into MergeBasicBlockIntoOnlyPred
> 	1545 entry block removals


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:953
+        // Succ cannot dominate itself.
+        if (*PI != Succ)
+          Updates.push_back({DominatorTree::Insert, *PI, Succ});
----------------
This is the same patch as D38383.


https://reviews.llvm.org/D38558