[llvm] r358741 - [MergeFunc] removeUsers: call remove() only on direct users

[Fangrui Song via llvm-commits]

Author: maskray
Date: Fri Apr 19 00:57:51 2019
New Revision: 358741

URL: http://llvm.org/viewvc/llvm-project?rev=358741&view=rev
Log:
[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.

Modified:
    llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
    llvm/trunk/test/Transforms/MergeFunc/unnamed-addr-reprocessing.ll

Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=358741&r1=358740&r2=358741&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Fri Apr 19 00:57:51 2019
@@ -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());
 }

Modified: llvm/trunk/test/Transforms/MergeFunc/unnamed-addr-reprocessing.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/unnamed-addr-reprocessing.ll?rev=358741&r1=358740&r2=358741&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/MergeFunc/unnamed-addr-reprocessing.ll (original)
+++ llvm/trunk/test/Transforms/MergeFunc/unnamed-addr-reprocessing.ll Fri Apr 19 00:57:51 2019
@@ -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