[RemoveDuplicatePHINodes] Start over after removing a PHI.

This makes RemoveDuplicatePHINodes more effective and fixes an assertion
failure. Triggering the assertions requires a DenseSet reallocation
so this change only contains a constructive test.

I'll explain the issue with a small example. In the following function
there's a duplicate PHI, %4 and %5 are identical. When this is found
the DenseSet in RemoveDuplicatePHINodes contains %2, %3 and %4.

define void @F() {
  br label %1

; <label>:1                                       ; preds = %1, %0
  %2 = phi i32 [ 42, %0 ], [ %4, %1 ]
  %3 = phi i32 [ 42, %0 ], [ %5, %1 ]
  %4 = phi i32 [ 42, %0 ], [ 23, %1 ]
  %5 = phi i32 [ 42, %0 ], [ 23, %1 ]
  br label %1
}

after RemoveDuplicatePHINodes runs the function looks like this. %3 has
changed and is now identical to %2, but RemoveDuplicatePHINodes never
saw this.

define void @F() {
  br label %1

; <label>:1                                       ; preds = %1, %0
  %2 = phi i32 [ 42, %0 ], [ %4, %1 ]
  %3 = phi i32 [ 42, %0 ], [ %4, %1 ]
  %4 = phi i32 [ 42, %0 ], [ 23, %1 ]
  br label %1
}

If the DenseSet does a reallocation now it will reinsert all
keys and stumble over %3 now having a different hash value than it had
when inserted into the map for the first time. This change clears the
set whenever a PHI is deleted and starts the progress from the
beginning, allowing %3 to be deleted and avoiding inconsistent DenseSet
state. This potentially has a negative performance impact because
it rescans all PHIs, but I don't think that this ever makes a difference
in practice.

llvm-svn: 246694

1 files changed, 5 insertions, 0 deletions

diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 8c29ed50b45..b0f729b2703 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -874,6 +874,11 @@ bool llvm::EliminateDuplicatePHINodes(BasicBlock *BB) {
       PN->replaceAllUsesWith(*Inserted.first);
       PN->eraseFromParent();
       Changed = true;
+
+      // The RAUW can change PHIs that we already visited. Start over from the
+      // beginning.
+      PHISet.clear();
+      I = BB->begin();
     }
   }