Skip to content

Commit

Permalink
[PM/LoopUnswitch] Add back a successor set that was removed based on
Browse files Browse the repository at this point in the history
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.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@331246 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
chandlerc committed May 1, 2018
1 parent 52c132f commit 112030e
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 112030e

Please sign in to comment.