[llvm] r331246 - [PM/LoopUnswitch] Add back a successor set that was removed based on

[Chandler Carruth via llvm-commits]

Author: chandlerc
Date: Tue May  1 02:42:09 2018
New Revision: 331246

URL: http://llvm.org/viewvc/llvm-project?rev=331246&view=rev
Log:
[PM/LoopUnswitch] Add back a successor set that was removed based on
code review.

It turns out this *is* necessary, and I read the comment on the API
correctly the first time. ;]

The `applyUpdates` routine requires that updates are "balanced". This is
in order to cleanly handle cycles like inserting, removing, nad then
re-inserting the same edge. This precludes inserting the same edge
multiple times in a row as handling that would cause the insertion logic
to become *ordered* instead of *unordered* (which is what the API
provides).

It happens that in this specific case nothing (other than an assert and
contract violation) goes wrong because we're never inserting and
removing the same edge. The implementation *happens* to do the right
thing to eliminate redundant insertions in that case.

But the requirement is there and there is an assert to catch it.
Somehow, after the code review I never did another asserts-clang build
testing loop-unswich for a long time. As a consequence, I didn't notice
this despite a bunch of testing going on, but it shows up immediately
with an asserts build of clang itself.

Modified:
    llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp?rev=331246&r1=331245&r2=331246&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp Tue May  1 02:42:09 2018
@@ -883,9 +883,13 @@ static BasicBlock *buildClonedLoopBlocks
             PN.removeIncomingValue(LoopBB, /*DeletePHIIfEmpty*/ false);
 
   // Record the domtree updates for the new blocks.
-  for (auto *ClonedBB : NewBlocks)
+  SmallPtrSet<BasicBlock *, 4> SuccSet;
+  for (auto *ClonedBB : NewBlocks) {
     for (auto *SuccBB : successors(ClonedBB))
-      DTUpdates.push_back({DominatorTree::Insert, ClonedBB, SuccBB});
+      if (SuccSet.insert(SuccBB).second)
+        DTUpdates.push_back({DominatorTree::Insert, ClonedBB, SuccBB});
+    SuccSet.clear();
+  }
 
   return ClonedPH;
 }