llvm.org GIT mirror llvm / 489d180
Merging r246694: ------------------------------------------------------------------------ r246694 | benny.kra | 2015-09-02 15:52:23 -0400 (Wed, 02 Sep 2015) | 44 lines [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. ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_37@252938 91177308-0d34-0410-b5e6-96231b3b80d8 Tom Stellard 3 years ago
2 changed file(s) with 42 addition(s) and 0 deletion(s). Raw diff Collapse all Expand all
868868 PN->replaceAllUsesWith(*Inserted.first);
869869 PN->eraseFromParent();
870870 Changed = true;
871
872 // The RAUW can change PHIs that we already visited. Start over from the
873 // beginning.
874 PHISet.clear();
875 I = BB->begin();
871876 }
872877 }
873878
5757 delete bb0;
5858 delete bb1;
5959 }
60
61 TEST(Local, RemoveDuplicatePHINodes) {
62 LLVMContext &C(getGlobalContext());
63 IRBuilder<> B(C);
64
65 std::unique_ptr F(
66 Function::Create(FunctionType::get(B.getVoidTy(), false),
67 GlobalValue::ExternalLinkage, "F"));
68 BasicBlock *Entry(BasicBlock::Create(C, "", F.get()));
69 BasicBlock *BB(BasicBlock::Create(C, "", F.get()));
70 BranchInst::Create(BB, Entry);
71
72 B.SetInsertPoint(BB);
73
74 AssertingVH P1 = B.CreatePHI(Type::getInt32Ty(C), 2);
75 P1->addIncoming(B.getInt32(42), Entry);
76
77 PHINode *P2 = B.CreatePHI(Type::getInt32Ty(C), 2);
78 P2->addIncoming(B.getInt32(42), Entry);
79
80 AssertingVH P3 = B.CreatePHI(Type::getInt32Ty(C), 2);
81 P3->addIncoming(B.getInt32(42), Entry);
82 P3->addIncoming(B.getInt32(23), BB);
83
84 PHINode *P4 = B.CreatePHI(Type::getInt32Ty(C), 2);
85 P4->addIncoming(B.getInt32(42), Entry);
86 P4->addIncoming(B.getInt32(23), BB);
87
88 P1->addIncoming(P3, BB);
89 P2->addIncoming(P4, BB);
90 BranchInst::Create(BB, BB);
91
92 // Verify that we can eliminate PHIs that become duplicates after chaning PHIs
93 // downstream.
94 EXPECT_TRUE(EliminateDuplicatePHINodes(BB));
95 EXPECT_EQ(3U, BB->size());
96 }