From 884f557bb216030769d40c00f55977e262b761a4 Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@google.com>
Date: Fri, 19 Apr 2019 07:57:51 +0000
Subject: [PATCH] [MergeFunc] removeUsers: call remove() only on direct users

removeUsers uses a work list to collect indirect users and call remove()
on those functions. However it has a bug (`if (!Visited.insert(UU).second)`).

Actually, we don't have to collect indirect users.
After the merge of F and G, G's callers will be considered (added to
Deferred). If G's callers can be merged, G's callers' callers will be
considered.

Update the test unnamed-addr-reprocessing.ll to make it clear we can
still merge indirect callers.

llvm-svn: 358741
---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp         | 24 +++-------------------
 .../MergeFunc/unnamed-addr-reprocessing.ll         | 22 ++++++++++++++++----
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index 8a40f04..9b3bf85 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -948,25 +948,7 @@ void MergeFunctions::remove(Function *F) {
 // For each instruction used by the value, remove() the function that contains
 // the instruction. This should happen right before a call to RAUW.
 void MergeFunctions::removeUsers(Value *V) {
-  std::vector<Value *> Worklist;
-  Worklist.push_back(V);
-  SmallPtrSet<Value*, 8> Visited;
-  Visited.insert(V);
-  while (!Worklist.empty()) {
-    Value *V = Worklist.back();
-    Worklist.pop_back();
-
-    for (User *U : V->users()) {
-      if (Instruction *I = dyn_cast<Instruction>(U)) {
-        remove(I->getFunction());
-      } else if (isa<GlobalValue>(U)) {
-        // do nothing
-      } else if (Constant *C = dyn_cast<Constant>(U)) {
-        for (User *UU : C->users()) {
-          if (!Visited.insert(UU).second)
-            Worklist.push_back(UU);
-        }
-      }
-    }
-  }
+  for (User *U : V->users())
+    if (auto *I = dyn_cast<Instruction>(U))
+      remove(I->getFunction());
 }
diff --git a/llvm/test/Transforms/MergeFunc/unnamed-addr-reprocessing.ll b/llvm/test/Transforms/MergeFunc/unnamed-addr-reprocessing.ll
index 5902edc..4981b33 100644
--- a/llvm/test/Transforms/MergeFunc/unnamed-addr-reprocessing.ll
+++ b/llvm/test/Transforms/MergeFunc/unnamed-addr-reprocessing.ll
@@ -1,15 +1,17 @@
 ; RUN: opt -S -mergefunc < %s | FileCheck %s
 
-; After test3 and test4 have been merged, we should detect that
-; test1 and test2 can also be merged.
+; After the merge of test5 and test6, we can merge test3 and test4,
+; then test1 and test2.
 
+; CHECK: define void @test6() unnamed_addr
+; CHECK-NEXT: tail call void @test5()
 ; CHECK: define void @test4() unnamed_addr
 ; CHECK-NEXT: tail call void @test3()
 ; CHECK: define void @test2() unnamed_addr
 ; CHECK-NEXT: tail call void @test1()
 
 declare void @dummy()
-  
+
 define void @test1() unnamed_addr {
     call void @test3()
     call void @test3()
@@ -23,12 +25,24 @@ define void @test2() unnamed_addr {
 }
 
 define void @test3() unnamed_addr {
+    call void @test5()
+    call void @test5()
+    ret void
+}
+
+define void @test4() unnamed_addr {
+    call void @test6()
+    call void @test6()
+    ret void
+}
+
+define void @test5() unnamed_addr {
     call void @dummy()
     call void @dummy()
     ret void
 }
 
-define void @test4() unnamed_addr {
+define void @test6() unnamed_addr {
     call void @dummy()
     call void @dummy()
     ret void
-- 
2.7.4