[llvm] [MergeFunctions] Add support to run the pass over a set of function pointers (PR #111045)

Rafael Eckstein via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 28 05:48:22 PST 2024


https://github.com/Casperento updated https://github.com/llvm/llvm-project/pull/111045

>From 69e1cc7d02df7d04b341c87abaa84cf4b6ab309d Mon Sep 17 00:00:00 2001
From: Casperento <44746868+Casperento at users.noreply.github.com>
Date: Tue, 24 Sep 2024 16:45:59 -0300
Subject: [PATCH 01/11] new runOn method

remove templates

unit tests added

format

updated data structures

format
---
 .../llvm/Transforms/IPO/MergeFunctions.h      |   7 +
 llvm/lib/Transforms/IPO/MergeFunctions.cpp    |  63 +++-
 .../unittests/Transforms/Utils/CMakeLists.txt |   1 +
 .../Transforms/Utils/MergeFunctionsTest.cpp   | 271 ++++++++++++++++++
 .../llvm/unittests/Transforms/Utils/BUILD.gn  |   1 +
 5 files changed, 341 insertions(+), 2 deletions(-)
 create mode 100644 llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp

diff --git a/llvm/include/llvm/Transforms/IPO/MergeFunctions.h b/llvm/include/llvm/Transforms/IPO/MergeFunctions.h
index 822f0fd99188d0..1b3b1d22f11e28 100644
--- a/llvm/include/llvm/Transforms/IPO/MergeFunctions.h
+++ b/llvm/include/llvm/Transforms/IPO/MergeFunctions.h
@@ -15,7 +15,10 @@
 #ifndef LLVM_TRANSFORMS_IPO_MERGEFUNCTIONS_H
 #define LLVM_TRANSFORMS_IPO_MERGEFUNCTIONS_H
 
+#include "llvm/IR/Function.h"
 #include "llvm/IR/PassManager.h"
+#include <map>
+#include <set>
 
 namespace llvm {
 
@@ -25,6 +28,10 @@ class Module;
 class MergeFunctionsPass : public PassInfoMixin<MergeFunctionsPass> {
 public:
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
+
+  static bool runOnModule(Module &M);
+  static std::pair<bool, std::map<Function *, Function *>>
+  runOnFunctions(std::set<Function *> &F);
 };
 
 } // end namespace llvm
diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index b50a700e09038f..a434d7920b6ccf 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -123,6 +123,7 @@
 #include <algorithm>
 #include <cassert>
 #include <iterator>
+#include <map>
 #include <set>
 #include <utility>
 #include <vector>
@@ -198,6 +199,8 @@ class MergeFunctions {
   }
 
   bool runOnModule(Module &M);
+  bool runOnFunctions(std::set<Function *> &F);
+  std::map<Function *, Function *> &getDelToNewMap();
 
 private:
   // The function comparison operator is provided here so that FunctionNodes do
@@ -298,17 +301,31 @@ class MergeFunctions {
   // dangling iterators into FnTree. The invariant that preserves this is that
   // there is exactly one mapping F -> FN for each FunctionNode FN in FnTree.
   DenseMap<AssertingVH<Function>, FnTreeType::iterator> FNodesInTree;
+
+  /// Deleted-New functions mapping
+  std::map<Function *, Function *> DelToNewMap;
 };
 } // end anonymous namespace
 
 PreservedAnalyses MergeFunctionsPass::run(Module &M,
                                           ModuleAnalysisManager &AM) {
-  MergeFunctions MF;
-  if (!MF.runOnModule(M))
+  if (!MergeFunctionsPass::runOnModule(M))
     return PreservedAnalyses::all();
   return PreservedAnalyses::none();
 }
 
+bool MergeFunctionsPass::runOnModule(Module &M) {
+  MergeFunctions MF;
+  return MF.runOnModule(M);
+}
+
+std::pair<bool, std::map<Function *, Function *>>
+MergeFunctionsPass::runOnFunctions(std::set<Function *> &F) {
+  MergeFunctions MF;
+  bool MergeResult = MF.runOnFunctions(F);
+  return {MergeResult, MF.getDelToNewMap()};
+}
+
 #ifndef NDEBUG
 bool MergeFunctions::doFunctionalCheck(std::vector<WeakTrackingVH> &Worklist) {
   if (const unsigned Max = NumFunctionsForVerificationCheck) {
@@ -468,6 +485,47 @@ bool MergeFunctions::runOnModule(Module &M) {
   return Changed;
 }
 
+bool MergeFunctions::runOnFunctions(std::set<Function *> &F) {
+  bool Changed = false;
+  std::vector<std::pair<IRHash, Function *>> HashedFuncs;
+  for (Function *Func : F) {
+    if (isEligibleForMerging(*Func)) {
+      HashedFuncs.push_back({StructuralHash(*Func), Func});
+    }
+  }
+  llvm::stable_sort(HashedFuncs, less_first());
+  auto S = HashedFuncs.begin();
+  for (auto I = HashedFuncs.begin(), IE = HashedFuncs.end(); I != IE; ++I) {
+    if ((I != S && std::prev(I)->first == I->first) ||
+        (std::next(I) != IE && std::next(I)->first == I->first)) {
+      Deferred.push_back(WeakTrackingVH(I->second));
+    }
+  }
+  do {
+    std::vector<WeakTrackingVH> Worklist;
+    Deferred.swap(Worklist);
+    LLVM_DEBUG(dbgs() << "size of function: " << F.size() << '\n');
+    LLVM_DEBUG(dbgs() << "size of worklist: " << Worklist.size() << '\n');
+    for (WeakTrackingVH &I : Worklist) {
+      if (!I)
+        continue;
+      Function *F = cast<Function>(I);
+      if (!F->isDeclaration() && !F->hasAvailableExternallyLinkage()) {
+        Changed |= insert(F);
+      }
+    }
+    LLVM_DEBUG(dbgs() << "size of FnTree: " << FnTree.size() << '\n');
+  } while (!Deferred.empty());
+  FnTree.clear();
+  FNodesInTree.clear();
+  GlobalNumbers.clear();
+  return Changed;
+}
+
+std::map<Function *, Function *> &MergeFunctions::getDelToNewMap() {
+  return this->DelToNewMap;
+}
+
 // Replace direct callers of Old with New.
 void MergeFunctions::replaceDirectCallers(Function *Old, Function *New) {
   for (Use &U : llvm::make_early_inc_range(Old->uses())) {
@@ -1004,6 +1062,7 @@ bool MergeFunctions::insert(Function *NewFunction) {
 
   Function *DeleteF = NewFunction;
   mergeTwoFunctions(OldF.getFunc(), DeleteF);
+  this->DelToNewMap.emplace(DeleteF, OldF.getFunc());
   return true;
 }
 
diff --git a/llvm/unittests/Transforms/Utils/CMakeLists.txt b/llvm/unittests/Transforms/Utils/CMakeLists.txt
index 5c7ec28709c169..7effa5d8e7d6d2 100644
--- a/llvm/unittests/Transforms/Utils/CMakeLists.txt
+++ b/llvm/unittests/Transforms/Utils/CMakeLists.txt
@@ -26,6 +26,7 @@ add_llvm_unittest(UtilsTests
   LoopUtilsTest.cpp
   MemTransferLowering.cpp
   ModuleUtilsTest.cpp
+  MergeFunctionsTest.cpp
   ScalarEvolutionExpanderTest.cpp
   SizeOptsTest.cpp
   SSAUpdaterBulkTest.cpp
diff --git a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
new file mode 100644
index 00000000000000..696c5391ef4f68
--- /dev/null
+++ b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
@@ -0,0 +1,271 @@
+//===- MergeFunctionsTest.cpp - Unit tests for
+//MergeFunctionsPass-----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Transforms/IPO/MergeFunctions.h"
+#include "llvm/AsmParser/Parser.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
+#include "llvm/Support/SourceMgr.h"
+#include "gtest/gtest.h"
+#include <memory>
+
+using namespace llvm;
+
+namespace {
+
+TEST(MergeFunctions, TrueOutputModuleTest) {
+  LLVMContext Ctx;
+  SMDiagnostic Err;
+  std::unique_ptr<Module> M(parseAssemblyString(R"invalid(
+        @.str = private unnamed_addr constant [10 x i8] c"On f: %d\0A\00", align 1
+        @.str.1 = private unnamed_addr constant [13 x i8] c"On main: %d\0A\00", align 1
+
+        define dso_local i32 @f(i32 noundef %arg) #0 {
+            entry:
+                %add109 = call i32 @_slice_add10(i32 %arg)
+                %call = call i32 (ptr, ...) @printf(ptr noundef @.str, i32 noundef %add109)
+                ret i32 %add109
+        }
+
+        declare i32 @printf(ptr noundef, ...) #1
+
+        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) #0 {
+            entry:
+                %add99 = call i32 @_slice_add10(i32 %argc)
+                %call = call i32 @f(i32 noundef 2)
+                %sub = sub nsw i32 %call, 6
+                %call10 = call i32 (ptr, ...) @printf(ptr noundef @.str.1, i32 noundef %add99)
+                ret i32 %add99
+        }
+
+        define internal i32 @_slice_add10(i32 %arg) #2 {
+            sliceclone_entry:
+                %0 = mul nsw i32 %arg, %arg
+                %1 = mul nsw i32 %0, 2
+                %2 = mul nsw i32 %1, 2
+                %3 = mul nsw i32 %2, 2
+                %4 = add nsw i32 %3, 2
+                ret i32 %4
+        }
+
+        define internal i32 @_slice_add10_alt(i32 %arg) #2 {
+            sliceclone_entry:
+                %0 = mul nsw i32 %arg, %arg
+                %1 = mul nsw i32 %0, 2
+                %2 = mul nsw i32 %1, 2
+                %3 = mul nsw i32 %2, 2
+                %4 = add nsw i32 %3, 2
+                ret i32 %4
+        }
+
+        attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+        attributes #1 = { "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+        attributes #2 = { nounwind willreturn }
+    )invalid",
+                                                Err, Ctx));
+
+  // Expects true after merging _slice_add10 and _slice_add10_alt
+  EXPECT_TRUE(MergeFunctionsPass::runOnModule(*M));
+}
+
+TEST(MergeFunctions, TrueOutputFunctionsTest) {
+  LLVMContext Ctx;
+  SMDiagnostic Err;
+  std::unique_ptr<Module> M(parseAssemblyString(R"invalid(
+        @.str = private unnamed_addr constant [10 x i8] c"On f: %d\0A\00", align 1
+        @.str.1 = private unnamed_addr constant [13 x i8] c"On main: %d\0A\00", align 1
+
+        define dso_local i32 @f(i32 noundef %arg) #0 {
+            entry:
+                %add109 = call i32 @_slice_add10(i32 %arg)
+                %call = call i32 (ptr, ...) @printf(ptr noundef @.str, i32 noundef %add109)
+                ret i32 %add109
+        }
+
+        declare i32 @printf(ptr noundef, ...) #1
+
+        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) #0 {
+            entry:
+                %add99 = call i32 @_slice_add10(i32 %argc)
+                %call = call i32 @f(i32 noundef 2)
+                %sub = sub nsw i32 %call, 6
+                %call10 = call i32 (ptr, ...) @printf(ptr noundef @.str.1, i32 noundef %add99)
+                ret i32 %add99
+        }
+
+        define internal i32 @_slice_add10(i32 %arg) #2 {
+            sliceclone_entry:
+                %0 = mul nsw i32 %arg, %arg
+                %1 = mul nsw i32 %0, 2
+                %2 = mul nsw i32 %1, 2
+                %3 = mul nsw i32 %2, 2
+                %4 = add nsw i32 %3, 2
+                ret i32 %4
+        }
+
+        define internal i32 @_slice_add10_alt(i32 %arg) #2 {
+            sliceclone_entry:
+                %0 = mul nsw i32 %arg, %arg
+                %1 = mul nsw i32 %0, 2
+                %2 = mul nsw i32 %1, 2
+                %3 = mul nsw i32 %2, 2
+                %4 = add nsw i32 %3, 2
+                ret i32 %4
+        }
+
+        attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+        attributes #1 = { "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+        attributes #2 = { nounwind willreturn }
+    )invalid",
+                                                Err, Ctx));
+
+  std::set<Function *> FunctionsSet;
+  for (Function &F : *M)
+    FunctionsSet.insert(&F);
+
+  std::pair<bool, std::map<Function *, Function *>> MergeResult =
+      MergeFunctionsPass::runOnFunctions(FunctionsSet);
+
+  // Expects true after merging _slice_add10 and _slice_add10_alt
+  EXPECT_TRUE(MergeResult.first);
+
+  // Expects that both functions (_slice_add10 and _slice_add10_alt)
+  // be mapped to the same new function
+  EXPECT_TRUE(MergeResult.second.size() > 0);
+  std::map<Function *, Function *> DelToNew = MergeResult.second;
+  Function *NewFunction = M->getFunction("_slice_add10");
+  for (auto P : DelToNew)
+    if (P.second)
+      EXPECT_EQ(P.second, NewFunction);
+}
+
+TEST(MergeFunctions, FalseOutputModuleTest) {
+  LLVMContext Ctx;
+  SMDiagnostic Err;
+  std::unique_ptr<Module> M(parseAssemblyString(R"invalid(
+        @.str = private unnamed_addr constant [10 x i8] c"On f: %d\0A\00", align 1
+        @.str.1 = private unnamed_addr constant [13 x i8] c"On main: %d\0A\00", align 1
+
+        define dso_local i32 @f(i32 noundef %arg) #0 {
+            entry:
+                %add109 = call i32 @_slice_add10(i32 %arg)
+                %call = call i32 (ptr, ...) @printf(ptr noundef @.str, i32 noundef %add109)
+                ret i32 %add109
+        }
+
+        declare i32 @printf(ptr noundef, ...) #1
+
+        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) #0 {
+            entry:
+                %add99 = call i32 @_slice_add10(i32 %argc)
+                %call = call i32 @f(i32 noundef 2)
+                %sub = sub nsw i32 %call, 6
+                %call10 = call i32 (ptr, ...) @printf(ptr noundef @.str.1, i32 noundef %add99)
+                ret i32 %add99
+        }
+
+        define internal i32 @_slice_add10(i32 %arg) #2 {
+            sliceclone_entry:
+                %0 = mul nsw i32 %arg, %arg
+                %1 = mul nsw i32 %0, 2
+                %2 = mul nsw i32 %1, 2
+                %3 = mul nsw i32 %2, 2
+                %4 = add nsw i32 %3, 2
+                ret i32 %4
+        }
+
+        define internal i32 @_slice_add10_alt(i32 %arg) #2 {
+            sliceclone_entry:
+                %0 = mul nsw i32 %arg, %arg
+                %1 = mul nsw i32 %0, 2
+                %2 = mul nsw i32 %1, 2
+                %3 = mul nsw i32 %2, 2
+                %4 = add nsw i32 %3, 2
+                ret i32 %0
+        }
+
+        attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+        attributes #1 = { "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+        attributes #2 = { nounwind willreturn }
+    )invalid",
+                                                Err, Ctx));
+
+  // Expects false after trying to merge _slice_add10 and _slice_add10_alt
+  EXPECT_FALSE(MergeFunctionsPass::runOnModule(*M));
+}
+
+TEST(MergeFunctions, FalseOutputFunctionsTest) {
+  LLVMContext Ctx;
+  SMDiagnostic Err;
+  std::unique_ptr<Module> M(parseAssemblyString(R"invalid(
+        @.str = private unnamed_addr constant [10 x i8] c"On f: %d\0A\00", align 1
+        @.str.1 = private unnamed_addr constant [13 x i8] c"On main: %d\0A\00", align 1
+
+        define dso_local i32 @f(i32 noundef %arg) #0 {
+            entry:
+                %add109 = call i32 @_slice_add10(i32 %arg)
+                %call = call i32 (ptr, ...) @printf(ptr noundef @.str, i32 noundef %add109)
+                ret i32 %add109
+        }
+
+        declare i32 @printf(ptr noundef, ...) #1
+
+        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) #0 {
+            entry:
+                %add99 = call i32 @_slice_add10(i32 %argc)
+                %call = call i32 @f(i32 noundef 2)
+                %sub = sub nsw i32 %call, 6
+                %call10 = call i32 (ptr, ...) @printf(ptr noundef @.str.1, i32 noundef %add99)
+                ret i32 %add99
+        }
+
+        define internal i32 @_slice_add10(i32 %arg) #2 {
+            sliceclone_entry:
+                %0 = mul nsw i32 %arg, %arg
+                %1 = mul nsw i32 %0, 2
+                %2 = mul nsw i32 %1, 2
+                %3 = mul nsw i32 %2, 2
+                %4 = add nsw i32 %3, 2
+                ret i32 %4
+        }
+
+        define internal i32 @_slice_add10_alt(i32 %arg) #2 {
+            sliceclone_entry:
+                %0 = mul nsw i32 %arg, %arg
+                %1 = mul nsw i32 %0, 2
+                %2 = mul nsw i32 %1, 2
+                %3 = mul nsw i32 %2, 2
+                %4 = add nsw i32 %3, 2
+                ret i32 %0
+        }
+
+        attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+        attributes #1 = { "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+        attributes #2 = { nounwind willreturn }
+    )invalid",
+                                                Err, Ctx));
+
+  std::set<Function *> FunctionsSet;
+  for (Function &F : *M)
+    FunctionsSet.insert(&F);
+
+  std::pair<bool, std::map<Function *, Function *>> MergeResult =
+      MergeFunctionsPass::runOnFunctions(FunctionsSet);
+
+  for (auto P : MergeResult.second)
+    std::cout << P.first << " " << P.second << "\n";
+
+  // Expects false after trying to merge _slice_add10 and _slice_add10_alt
+  EXPECT_FALSE(MergeResult.first);
+
+  // Expects empty map
+  EXPECT_EQ(MergeResult.second.size(), 0u);
+}
+
+} // namespace
\ No newline at end of file
diff --git a/llvm/utils/gn/secondary/llvm/unittests/Transforms/Utils/BUILD.gn b/llvm/utils/gn/secondary/llvm/unittests/Transforms/Utils/BUILD.gn
index 380ed71a2bc010..fcea55c91f083c 100644
--- a/llvm/utils/gn/secondary/llvm/unittests/Transforms/Utils/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/unittests/Transforms/Utils/BUILD.gn
@@ -27,6 +27,7 @@ unittest("UtilsTests") {
     "LoopUtilsTest.cpp",
     "MemTransferLowering.cpp",
     "ModuleUtilsTest.cpp",
+    "MergeFunctionsTest.cpp",
     "ProfDataUtilTest.cpp",
     "SSAUpdaterBulkTest.cpp",
     "ScalarEvolutionExpanderTest.cpp",

>From 3df9ca11f09f94f2182725e4ea6fcaa24dae0da4 Mon Sep 17 00:00:00 2001
From: Casperento <44746868+Casperento at users.noreply.github.com>
Date: Sat, 2 Nov 2024 12:05:25 -0300
Subject: [PATCH 02/11] fix format and stable_hash

---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp             | 2 +-
 llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index 89fa78992719c4..03a6e925f3f303 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -487,7 +487,7 @@ bool MergeFunctions::runOnModule(Module &M) {
 
 bool MergeFunctions::runOnFunctions(std::set<Function *> &F) {
   bool Changed = false;
-  std::vector<std::pair<IRHash, Function *>> HashedFuncs;
+  std::vector<std::pair<stable_hash, Function *>> HashedFuncs;
   for (Function *Func : F) {
     if (isEligibleForMerging(*Func)) {
       HashedFuncs.push_back({StructuralHash(*Func), Func});
diff --git a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
index 696c5391ef4f68..2f86ec6ed77fcd 100644
--- a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
@@ -1,5 +1,5 @@
 //===- MergeFunctionsTest.cpp - Unit tests for
-//MergeFunctionsPass-----------===//
+// MergeFunctionsPass-----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

>From 67d70af9d4e0f1d09f77d8eae77ea5c420bc91df Mon Sep 17 00:00:00 2001
From: Casperento <44746868+Casperento at users.noreply.github.com>
Date: Wed, 6 Nov 2024 12:30:34 -0300
Subject: [PATCH 03/11] fix comments

---
 .../llvm/Transforms/IPO/MergeFunctions.h      |  3 +-
 llvm/lib/Transforms/IPO/MergeFunctions.cpp    | 98 ++++++++-----------
 .../Transforms/Utils/MergeFunctionsTest.cpp   |  6 +-
 3 files changed, 43 insertions(+), 64 deletions(-)

diff --git a/llvm/include/llvm/Transforms/IPO/MergeFunctions.h b/llvm/include/llvm/Transforms/IPO/MergeFunctions.h
index 1b3b1d22f11e28..f2b27391dc3684 100644
--- a/llvm/include/llvm/Transforms/IPO/MergeFunctions.h
+++ b/llvm/include/llvm/Transforms/IPO/MergeFunctions.h
@@ -17,7 +17,6 @@
 
 #include "llvm/IR/Function.h"
 #include "llvm/IR/PassManager.h"
-#include <map>
 #include <set>
 
 namespace llvm {
@@ -30,7 +29,7 @@ class MergeFunctionsPass : public PassInfoMixin<MergeFunctionsPass> {
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
 
   static bool runOnModule(Module &M);
-  static std::pair<bool, std::map<Function *, Function *>>
+  static std::pair<bool, DenseMap<Function *, Function *>>
   runOnFunctions(std::set<Function *> &F);
 };
 
diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index b4ed3cefa2028a..1ef267d8bb3ce9 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -105,6 +105,7 @@
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
+#include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/StructuralHash.h"
 #include "llvm/IR/Type.h"
@@ -117,6 +118,7 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/IPO.h"
+#include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/Transforms/Utils/FunctionComparator.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include <algorithm>
@@ -197,9 +199,10 @@ class MergeFunctions {
   MergeFunctions() : FnTree(FunctionNodeCmp(&GlobalNumbers)) {
   }
 
-  bool runOnModule(Module &M);
-  bool runOnFunctions(std::set<Function *> &F);
-  std::map<Function *, Function *> &getDelToNewMap();
+  template <typename FuncContainer>
+  bool runOnModule(FuncContainer &Functions);
+  std::pair<bool, DenseMap<Function *, Function *>>
+  runOnFunctions(std::set<Function *> &F);
 
 private:
   // The function comparison operator is provided here so that FunctionNodes do
@@ -302,7 +305,7 @@ class MergeFunctions {
   DenseMap<AssertingVH<Function>, FnTreeType::iterator> FNodesInTree;
 
   /// Deleted-New functions mapping
-  std::map<Function *, Function *> DelToNewMap;
+  DenseMap<Function *, Function *> DelToNewMap;
 };
 } // end anonymous namespace
 
@@ -318,11 +321,10 @@ bool MergeFunctionsPass::runOnModule(Module &M) {
   return MF.runOnModule(M);
 }
 
-std::pair<bool, std::map<Function *, Function *>>
+std::pair<bool, DenseMap<Function *, Function *>>
 MergeFunctionsPass::runOnFunctions(std::set<Function *> &F) {
   MergeFunctions MF;
-  bool MergeResult = MF.runOnFunctions(F);
-  return {MergeResult, MF.getDelToNewMap()};
+  return MF.runOnFunctions(F);
 }
 
 #ifndef NDEBUG
@@ -426,24 +428,36 @@ static bool isEligibleForMerging(Function &F) {
          !hasDistinctMetadataIntrinsic(F);
 }
 
-bool MergeFunctions::runOnModule(Module &M) {
+template <typename FuncContainer>
+bool MergeFunctions::runOnModule(FuncContainer &M) {
   bool Changed = false;
 
-  SmallVector<GlobalValue *, 4> UsedV;
-  collectUsedGlobalVariables(M, UsedV, /*CompilerUsed=*/false);
-  collectUsedGlobalVariables(M, UsedV, /*CompilerUsed=*/true);
-  Used.insert(UsedV.begin(), UsedV.end());
+  if constexpr (std::is_same<FuncContainer, Module>::value) {
+    SmallVector<GlobalValue *, 4> UsedV;
+    collectUsedGlobalVariables(M, UsedV, /*CompilerUsed=*/false);
+    collectUsedGlobalVariables(M, UsedV, /*CompilerUsed=*/true);
+    Used.insert(UsedV.begin(), UsedV.end());
+  }
 
   // All functions in the module, ordered by hash. Functions with a unique
   // hash value are easily eliminated.
   std::vector<std::pair<stable_hash, Function *>> HashedFuncs;
-  for (Function &Func : M) {
-    if (isEligibleForMerging(Func)) {
-      HashedFuncs.push_back({StructuralHash(Func), &Func});
+  if constexpr (std::is_same<FuncContainer, std::set<Function *>>::value) {
+    for (Function *Func : M) {
+      if (isEligibleForMerging(*Func)) {
+        HashedFuncs.push_back({StructuralHash(*Func), Func});
+      }
+    }
+  }
+  if constexpr (std::is_same<FuncContainer, Module>::value) {
+    for (Function &Func : M) {
+      if (isEligibleForMerging(Func)) {
+        HashedFuncs.push_back({StructuralHash(Func), &Func});
+      }
     }
   }
 
-  llvm::stable_sort(HashedFuncs, less_first());
+  stable_sort(HashedFuncs, less_first());
 
   auto S = HashedFuncs.begin();
   for (auto I = HashedFuncs.begin(), IE = HashedFuncs.end(); I != IE; ++I) {
@@ -484,50 +498,16 @@ bool MergeFunctions::runOnModule(Module &M) {
   return Changed;
 }
 
-bool MergeFunctions::runOnFunctions(std::set<Function *> &F) {
-  bool Changed = false;
-  std::vector<std::pair<stable_hash, Function *>> HashedFuncs;
-  for (Function *Func : F) {
-    if (isEligibleForMerging(*Func)) {
-      HashedFuncs.push_back({StructuralHash(*Func), Func});
-    }
-  }
-  llvm::stable_sort(HashedFuncs, less_first());
-  auto S = HashedFuncs.begin();
-  for (auto I = HashedFuncs.begin(), IE = HashedFuncs.end(); I != IE; ++I) {
-    if ((I != S && std::prev(I)->first == I->first) ||
-        (std::next(I) != IE && std::next(I)->first == I->first)) {
-      Deferred.push_back(WeakTrackingVH(I->second));
-    }
-  }
-  do {
-    std::vector<WeakTrackingVH> Worklist;
-    Deferred.swap(Worklist);
-    LLVM_DEBUG(dbgs() << "size of function: " << F.size() << '\n');
-    LLVM_DEBUG(dbgs() << "size of worklist: " << Worklist.size() << '\n');
-    for (WeakTrackingVH &I : Worklist) {
-      if (!I)
-        continue;
-      Function *F = cast<Function>(I);
-      if (!F->isDeclaration() && !F->hasAvailableExternallyLinkage()) {
-        Changed |= insert(F);
-      }
-    }
-    LLVM_DEBUG(dbgs() << "size of FnTree: " << FnTree.size() << '\n');
-  } while (!Deferred.empty());
-  FnTree.clear();
-  FNodesInTree.clear();
-  GlobalNumbers.clear();
-  return Changed;
-}
-
-std::map<Function *, Function *> &MergeFunctions::getDelToNewMap() {
-  return this->DelToNewMap;
+std::pair<bool, DenseMap<Function *, Function *>>
+MergeFunctions::runOnFunctions(std::set<Function *> &F) {
+  bool MergeResult = false;
+  MergeResult = this->runOnModule(F);
+  return {MergeResult, this->DelToNewMap};
 }
 
 // Replace direct callers of Old with New.
 void MergeFunctions::replaceDirectCallers(Function *Old, Function *New) {
-  for (Use &U : llvm::make_early_inc_range(Old->uses())) {
+  for (Use &U : make_early_inc_range(Old->uses())) {
     CallBase *CB = dyn_cast<CallBase>(U.getUser());
     if (CB && CB->isCallee(&U)) {
       // Do not copy attributes from the called function to the call-site.
@@ -826,8 +806,8 @@ void MergeFunctions::writeThunk(Function *F, Function *G) {
   ReturnInst *RI = nullptr;
   bool isSwiftTailCall = F->getCallingConv() == CallingConv::SwiftTail &&
                          G->getCallingConv() == CallingConv::SwiftTail;
-  CI->setTailCallKind(isSwiftTailCall ? llvm::CallInst::TCK_MustTail
-                                      : llvm::CallInst::TCK_Tail);
+  CI->setTailCallKind(isSwiftTailCall ? CallInst::TCK_MustTail
+                                      : CallInst::TCK_Tail);
   CI->setCallingConv(F->getCallingConv());
   CI->setAttributes(F->getAttributes());
   if (H->getReturnType()->isVoidTy()) {
@@ -1061,7 +1041,7 @@ bool MergeFunctions::insert(Function *NewFunction) {
 
   Function *DeleteF = NewFunction;
   mergeTwoFunctions(OldF.getFunc(), DeleteF);
-  this->DelToNewMap.emplace(DeleteF, OldF.getFunc());
+  this->DelToNewMap.insert({DeleteF, OldF.getFunc()});
   return true;
 }
 
diff --git a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
index 2f86ec6ed77fcd..e1d2189d9d1dcf 100644
--- a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
@@ -129,7 +129,7 @@ TEST(MergeFunctions, TrueOutputFunctionsTest) {
   for (Function &F : *M)
     FunctionsSet.insert(&F);
 
-  std::pair<bool, std::map<Function *, Function *>> MergeResult =
+  std::pair<bool, DenseMap<Function *, Function *>> MergeResult =
       MergeFunctionsPass::runOnFunctions(FunctionsSet);
 
   // Expects true after merging _slice_add10 and _slice_add10_alt
@@ -138,7 +138,7 @@ TEST(MergeFunctions, TrueOutputFunctionsTest) {
   // Expects that both functions (_slice_add10 and _slice_add10_alt)
   // be mapped to the same new function
   EXPECT_TRUE(MergeResult.second.size() > 0);
-  std::map<Function *, Function *> DelToNew = MergeResult.second;
+  DenseMap<Function *, Function *> DelToNew = MergeResult.second;
   Function *NewFunction = M->getFunction("_slice_add10");
   for (auto P : DelToNew)
     if (P.second)
@@ -255,7 +255,7 @@ TEST(MergeFunctions, FalseOutputFunctionsTest) {
   for (Function &F : *M)
     FunctionsSet.insert(&F);
 
-  std::pair<bool, std::map<Function *, Function *>> MergeResult =
+  std::pair<bool, DenseMap<Function *, Function *>> MergeResult =
       MergeFunctionsPass::runOnFunctions(FunctionsSet);
 
   for (auto P : MergeResult.second)

>From eae748a33a7c96230cb392d4fe8352af760a687f Mon Sep 17 00:00:00 2001
From: Casperento <44746868+Casperento at users.noreply.github.com>
Date: Fri, 15 Nov 2024 16:18:31 -0300
Subject: [PATCH 04/11] fix comments

---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp    | 46 +++++++--------
 .../Transforms/Utils/MergeFunctionsTest.cpp   | 59 +++++++------------
 2 files changed, 43 insertions(+), 62 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index 1ef267d8bb3ce9..ce974b6ea04e8b 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -200,10 +200,12 @@ class MergeFunctions {
   }
 
   template <typename FuncContainer>
-  bool runOnModule(FuncContainer &Functions);
+  bool run(FuncContainer &Functions);
   std::pair<bool, DenseMap<Function *, Function *>>
   runOnFunctions(std::set<Function *> &F);
 
+  SmallPtrSet<GlobalValue *, 4>& getUsed();
+
 private:
   // The function comparison operator is provided here so that FunctionNodes do
   // not need to become larger with another pointer.
@@ -316,9 +318,17 @@ PreservedAnalyses MergeFunctionsPass::run(Module &M,
   return PreservedAnalyses::none();
 }
 
+SmallPtrSet<GlobalValue *, 4>& MergeFunctions::getUsed() {
+  return Used;
+}
+
 bool MergeFunctionsPass::runOnModule(Module &M) {
   MergeFunctions MF;
-  return MF.runOnModule(M);
+  SmallVector<GlobalValue *, 4> UsedV;
+  collectUsedGlobalVariables(M, UsedV, /*CompilerUsed=*/false);
+  collectUsedGlobalVariables(M, UsedV, /*CompilerUsed=*/true);
+  MF.getUsed().insert(UsedV.begin(), UsedV.end());
+  return MF.run(M);
 }
 
 std::pair<bool, DenseMap<Function *, Function *>>
@@ -428,36 +438,24 @@ static bool isEligibleForMerging(Function &F) {
          !hasDistinctMetadataIntrinsic(F);
 }
 
+inline Function *asPtr(Function *Fn) { return Fn; }
+inline Function *asPtr(Function &Fn) { return &Fn; }
+
 template <typename FuncContainer>
-bool MergeFunctions::runOnModule(FuncContainer &M) {
+bool MergeFunctions::run(FuncContainer &M) {
   bool Changed = false;
 
-  if constexpr (std::is_same<FuncContainer, Module>::value) {
-    SmallVector<GlobalValue *, 4> UsedV;
-    collectUsedGlobalVariables(M, UsedV, /*CompilerUsed=*/false);
-    collectUsedGlobalVariables(M, UsedV, /*CompilerUsed=*/true);
-    Used.insert(UsedV.begin(), UsedV.end());
-  }
-
   // All functions in the module, ordered by hash. Functions with a unique
   // hash value are easily eliminated.
   std::vector<std::pair<stable_hash, Function *>> HashedFuncs;
-  if constexpr (std::is_same<FuncContainer, std::set<Function *>>::value) {
-    for (Function *Func : M) {
-      if (isEligibleForMerging(*Func)) {
-        HashedFuncs.push_back({StructuralHash(*Func), Func});
-      }
-    }
-  }
-  if constexpr (std::is_same<FuncContainer, Module>::value) {
-    for (Function &Func : M) {
-      if (isEligibleForMerging(Func)) {
-        HashedFuncs.push_back({StructuralHash(Func), &Func});
-      }
+  for (auto &Func : M) {
+    Function *FuncPtr = asPtr(Func);
+    if (isEligibleForMerging(*FuncPtr)) {
+      HashedFuncs.push_back({StructuralHash(*FuncPtr), FuncPtr});
     }
   }
 
-  stable_sort(HashedFuncs, less_first());
+  llvm::stable_sort(HashedFuncs, less_first());
 
   auto S = HashedFuncs.begin();
   for (auto I = HashedFuncs.begin(), IE = HashedFuncs.end(); I != IE; ++I) {
@@ -501,7 +499,7 @@ bool MergeFunctions::runOnModule(FuncContainer &M) {
 std::pair<bool, DenseMap<Function *, Function *>>
 MergeFunctions::runOnFunctions(std::set<Function *> &F) {
   bool MergeResult = false;
-  MergeResult = this->runOnModule(F);
+  MergeResult = this->run(F);
   return {MergeResult, this->DelToNewMap};
 }
 
diff --git a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
index e1d2189d9d1dcf..50d921a93429b0 100644
--- a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
@@ -1,5 +1,4 @@
-//===- MergeFunctionsTest.cpp - Unit tests for
-// MergeFunctionsPass-----------===//
+//===- MergeFunctionsTest.cpp - Unit tests for MergeFunctionsPass-----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -26,16 +25,16 @@ TEST(MergeFunctions, TrueOutputModuleTest) {
         @.str = private unnamed_addr constant [10 x i8] c"On f: %d\0A\00", align 1
         @.str.1 = private unnamed_addr constant [13 x i8] c"On main: %d\0A\00", align 1
 
-        define dso_local i32 @f(i32 noundef %arg) #0 {
+        define dso_local i32 @f(i32 noundef %arg) {
             entry:
                 %add109 = call i32 @_slice_add10(i32 %arg)
                 %call = call i32 (ptr, ...) @printf(ptr noundef @.str, i32 noundef %add109)
                 ret i32 %add109
         }
 
-        declare i32 @printf(ptr noundef, ...) #1
+        declare i32 @printf(ptr noundef, ...)
 
-        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) #0 {
+        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) {
             entry:
                 %add99 = call i32 @_slice_add10(i32 %argc)
                 %call = call i32 @f(i32 noundef 2)
@@ -44,7 +43,7 @@ TEST(MergeFunctions, TrueOutputModuleTest) {
                 ret i32 %add99
         }
 
-        define internal i32 @_slice_add10(i32 %arg) #2 {
+        define internal i32 @_slice_add10(i32 %arg) {
             sliceclone_entry:
                 %0 = mul nsw i32 %arg, %arg
                 %1 = mul nsw i32 %0, 2
@@ -54,7 +53,7 @@ TEST(MergeFunctions, TrueOutputModuleTest) {
                 ret i32 %4
         }
 
-        define internal i32 @_slice_add10_alt(i32 %arg) #2 {
+        define internal i32 @_slice_add10_alt(i32 %arg) {
             sliceclone_entry:
                 %0 = mul nsw i32 %arg, %arg
                 %1 = mul nsw i32 %0, 2
@@ -63,10 +62,6 @@ TEST(MergeFunctions, TrueOutputModuleTest) {
                 %4 = add nsw i32 %3, 2
                 ret i32 %4
         }
-
-        attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
-        attributes #1 = { "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
-        attributes #2 = { nounwind willreturn }
     )invalid",
                                                 Err, Ctx));
 
@@ -81,16 +76,16 @@ TEST(MergeFunctions, TrueOutputFunctionsTest) {
         @.str = private unnamed_addr constant [10 x i8] c"On f: %d\0A\00", align 1
         @.str.1 = private unnamed_addr constant [13 x i8] c"On main: %d\0A\00", align 1
 
-        define dso_local i32 @f(i32 noundef %arg) #0 {
+        define dso_local i32 @f(i32 noundef %arg) {
             entry:
                 %add109 = call i32 @_slice_add10(i32 %arg)
                 %call = call i32 (ptr, ...) @printf(ptr noundef @.str, i32 noundef %add109)
                 ret i32 %add109
         }
 
-        declare i32 @printf(ptr noundef, ...) #1
+        declare i32 @printf(ptr noundef, ...)
 
-        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) #0 {
+        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) {
             entry:
                 %add99 = call i32 @_slice_add10(i32 %argc)
                 %call = call i32 @f(i32 noundef 2)
@@ -99,7 +94,7 @@ TEST(MergeFunctions, TrueOutputFunctionsTest) {
                 ret i32 %add99
         }
 
-        define internal i32 @_slice_add10(i32 %arg) #2 {
+        define internal i32 @_slice_add10(i32 %arg) {
             sliceclone_entry:
                 %0 = mul nsw i32 %arg, %arg
                 %1 = mul nsw i32 %0, 2
@@ -109,7 +104,7 @@ TEST(MergeFunctions, TrueOutputFunctionsTest) {
                 ret i32 %4
         }
 
-        define internal i32 @_slice_add10_alt(i32 %arg) #2 {
+        define internal i32 @_slice_add10_alt(i32 %arg) {
             sliceclone_entry:
                 %0 = mul nsw i32 %arg, %arg
                 %1 = mul nsw i32 %0, 2
@@ -118,10 +113,6 @@ TEST(MergeFunctions, TrueOutputFunctionsTest) {
                 %4 = add nsw i32 %3, 2
                 ret i32 %4
         }
-
-        attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
-        attributes #1 = { "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
-        attributes #2 = { nounwind willreturn }
     )invalid",
                                                 Err, Ctx));
 
@@ -152,16 +143,16 @@ TEST(MergeFunctions, FalseOutputModuleTest) {
         @.str = private unnamed_addr constant [10 x i8] c"On f: %d\0A\00", align 1
         @.str.1 = private unnamed_addr constant [13 x i8] c"On main: %d\0A\00", align 1
 
-        define dso_local i32 @f(i32 noundef %arg) #0 {
+        define dso_local i32 @f(i32 noundef %arg) {
             entry:
                 %add109 = call i32 @_slice_add10(i32 %arg)
                 %call = call i32 (ptr, ...) @printf(ptr noundef @.str, i32 noundef %add109)
                 ret i32 %add109
         }
 
-        declare i32 @printf(ptr noundef, ...) #1
+        declare i32 @printf(ptr noundef, ...)
 
-        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) #0 {
+        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) {
             entry:
                 %add99 = call i32 @_slice_add10(i32 %argc)
                 %call = call i32 @f(i32 noundef 2)
@@ -170,7 +161,7 @@ TEST(MergeFunctions, FalseOutputModuleTest) {
                 ret i32 %add99
         }
 
-        define internal i32 @_slice_add10(i32 %arg) #2 {
+        define internal i32 @_slice_add10(i32 %arg) {
             sliceclone_entry:
                 %0 = mul nsw i32 %arg, %arg
                 %1 = mul nsw i32 %0, 2
@@ -180,7 +171,7 @@ TEST(MergeFunctions, FalseOutputModuleTest) {
                 ret i32 %4
         }
 
-        define internal i32 @_slice_add10_alt(i32 %arg) #2 {
+        define internal i32 @_slice_add10_alt(i32 %arg) {
             sliceclone_entry:
                 %0 = mul nsw i32 %arg, %arg
                 %1 = mul nsw i32 %0, 2
@@ -189,10 +180,6 @@ TEST(MergeFunctions, FalseOutputModuleTest) {
                 %4 = add nsw i32 %3, 2
                 ret i32 %0
         }
-
-        attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
-        attributes #1 = { "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
-        attributes #2 = { nounwind willreturn }
     )invalid",
                                                 Err, Ctx));
 
@@ -207,16 +194,16 @@ TEST(MergeFunctions, FalseOutputFunctionsTest) {
         @.str = private unnamed_addr constant [10 x i8] c"On f: %d\0A\00", align 1
         @.str.1 = private unnamed_addr constant [13 x i8] c"On main: %d\0A\00", align 1
 
-        define dso_local i32 @f(i32 noundef %arg) #0 {
+        define dso_local i32 @f(i32 noundef %arg) {
             entry:
                 %add109 = call i32 @_slice_add10(i32 %arg)
                 %call = call i32 (ptr, ...) @printf(ptr noundef @.str, i32 noundef %add109)
                 ret i32 %add109
         }
 
-        declare i32 @printf(ptr noundef, ...) #1
+        declare i32 @printf(ptr noundef, ...)
 
-        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) #0 {
+        define dso_local i32 @main(i32 noundef %argc, ptr noundef %argv) {
             entry:
                 %add99 = call i32 @_slice_add10(i32 %argc)
                 %call = call i32 @f(i32 noundef 2)
@@ -225,7 +212,7 @@ TEST(MergeFunctions, FalseOutputFunctionsTest) {
                 ret i32 %add99
         }
 
-        define internal i32 @_slice_add10(i32 %arg) #2 {
+        define internal i32 @_slice_add10(i32 %arg) {
             sliceclone_entry:
                 %0 = mul nsw i32 %arg, %arg
                 %1 = mul nsw i32 %0, 2
@@ -235,7 +222,7 @@ TEST(MergeFunctions, FalseOutputFunctionsTest) {
                 ret i32 %4
         }
 
-        define internal i32 @_slice_add10_alt(i32 %arg) #2 {
+        define internal i32 @_slice_add10_alt(i32 %arg) {
             sliceclone_entry:
                 %0 = mul nsw i32 %arg, %arg
                 %1 = mul nsw i32 %0, 2
@@ -244,10 +231,6 @@ TEST(MergeFunctions, FalseOutputFunctionsTest) {
                 %4 = add nsw i32 %3, 2
                 ret i32 %0
         }
-
-        attributes #0 = { noinline nounwind uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
-        attributes #1 = { "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
-        attributes #2 = { nounwind willreturn }
     )invalid",
                                                 Err, Ctx));
 

>From 1fdf94e3e77c25156e56db0d8cde5f7965fb7e2f Mon Sep 17 00:00:00 2001
From: Casperento <44746868+Casperento at users.noreply.github.com>
Date: Fri, 15 Nov 2024 16:21:41 -0300
Subject: [PATCH 05/11] fix format

---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp           | 12 ++++--------
 .../Transforms/Utils/MergeFunctionsTest.cpp          |  3 ++-
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index ce974b6ea04e8b..f80f5fcf769ac8 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -199,12 +199,11 @@ class MergeFunctions {
   MergeFunctions() : FnTree(FunctionNodeCmp(&GlobalNumbers)) {
   }
 
-  template <typename FuncContainer>
-  bool run(FuncContainer &Functions);
+  template <typename FuncContainer> bool run(FuncContainer &Functions);
   std::pair<bool, DenseMap<Function *, Function *>>
   runOnFunctions(std::set<Function *> &F);
 
-  SmallPtrSet<GlobalValue *, 4>& getUsed();
+  SmallPtrSet<GlobalValue *, 4> &getUsed();
 
 private:
   // The function comparison operator is provided here so that FunctionNodes do
@@ -318,9 +317,7 @@ PreservedAnalyses MergeFunctionsPass::run(Module &M,
   return PreservedAnalyses::none();
 }
 
-SmallPtrSet<GlobalValue *, 4>& MergeFunctions::getUsed() {
-  return Used;
-}
+SmallPtrSet<GlobalValue *, 4> &MergeFunctions::getUsed() { return Used; }
 
 bool MergeFunctionsPass::runOnModule(Module &M) {
   MergeFunctions MF;
@@ -441,8 +438,7 @@ static bool isEligibleForMerging(Function &F) {
 inline Function *asPtr(Function *Fn) { return Fn; }
 inline Function *asPtr(Function &Fn) { return &Fn; }
 
-template <typename FuncContainer>
-bool MergeFunctions::run(FuncContainer &M) {
+template <typename FuncContainer> bool MergeFunctions::run(FuncContainer &M) {
   bool Changed = false;
 
   // All functions in the module, ordered by hash. Functions with a unique
diff --git a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
index 50d921a93429b0..c3c70a0230bcad 100644
--- a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
@@ -1,4 +1,5 @@
-//===- MergeFunctionsTest.cpp - Unit tests for MergeFunctionsPass-----------===//
+//===- MergeFunctionsTest.cpp - Unit tests for
+//MergeFunctionsPass-----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

>From d193a639f05ad094987f61e2e59dd11daf234891 Mon Sep 17 00:00:00 2001
From: Casperento <44746868+Casperento at users.noreply.github.com>
Date: Fri, 15 Nov 2024 21:44:24 -0300
Subject: [PATCH 06/11] fix comment

---
 llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
index c3c70a0230bcad..a58ce6f41ec404 100644
--- a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
@@ -1,5 +1,4 @@
-//===- MergeFunctionsTest.cpp - Unit tests for
-//MergeFunctionsPass-----------===//
+//===- MergeFunctionsTest.cpp - Unit tests for MergeFunctionsPass ---------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.

>From 04c23d033a94aa767006948dd88963ba177478c0 Mon Sep 17 00:00:00 2001
From: Casperento <44746868+Casperento at users.noreply.github.com>
Date: Fri, 22 Nov 2024 00:14:32 -0300
Subject: [PATCH 07/11] unused includes removed

---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index f80f5fcf769ac8..fef94aa88e8a0b 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -105,7 +105,6 @@
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
-#include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/StructuralHash.h"
 #include "llvm/IR/Type.h"
@@ -118,13 +117,11 @@
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/IPO.h"
-#include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/Transforms/Utils/FunctionComparator.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include <algorithm>
 #include <cassert>
 #include <iterator>
-#include <map>
 #include <set>
 #include <utility>
 #include <vector>

>From 350395d7c6e7614e3012443abe155cfc1b307e87 Mon Sep 17 00:00:00 2001
From: Casperento <44746868+Casperento at users.noreply.github.com>
Date: Wed, 27 Nov 2024 12:19:00 -0300
Subject: [PATCH 08/11] comments fix

---
 .../llvm/Transforms/IPO/MergeFunctions.h      |  7 ++---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp    | 20 ++++++-------
 .../Transforms/Utils/MergeFunctionsTest.cpp   | 29 ++++++++-----------
 3 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/llvm/include/llvm/Transforms/IPO/MergeFunctions.h b/llvm/include/llvm/Transforms/IPO/MergeFunctions.h
index f2b27391dc3684..71f175c6472b44 100644
--- a/llvm/include/llvm/Transforms/IPO/MergeFunctions.h
+++ b/llvm/include/llvm/Transforms/IPO/MergeFunctions.h
@@ -15,13 +15,12 @@
 #ifndef LLVM_TRANSFORMS_IPO_MERGEFUNCTIONS_H
 #define LLVM_TRANSFORMS_IPO_MERGEFUNCTIONS_H
 
-#include "llvm/IR/Function.h"
 #include "llvm/IR/PassManager.h"
-#include <set>
 
 namespace llvm {
 
 class Module;
+class Function;
 
 /// Merge identical functions.
 class MergeFunctionsPass : public PassInfoMixin<MergeFunctionsPass> {
@@ -29,8 +28,8 @@ class MergeFunctionsPass : public PassInfoMixin<MergeFunctionsPass> {
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
 
   static bool runOnModule(Module &M);
-  static std::pair<bool, DenseMap<Function *, Function *>>
-  runOnFunctions(std::set<Function *> &F);
+  static DenseMap<Function *, Function *>
+  runOnFunctions(ArrayRef<Function *> F);
 };
 
 } // end namespace llvm
diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index fef94aa88e8a0b..ea013245b50148 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -197,8 +197,7 @@ class MergeFunctions {
   }
 
   template <typename FuncContainer> bool run(FuncContainer &Functions);
-  std::pair<bool, DenseMap<Function *, Function *>>
-  runOnFunctions(std::set<Function *> &F);
+  DenseMap<Function *, Function *> runOnFunctions(ArrayRef<Function *> F);
 
   SmallPtrSet<GlobalValue *, 4> &getUsed();
 
@@ -325,8 +324,8 @@ bool MergeFunctionsPass::runOnModule(Module &M) {
   return MF.run(M);
 }
 
-std::pair<bool, DenseMap<Function *, Function *>>
-MergeFunctionsPass::runOnFunctions(std::set<Function *> &F) {
+DenseMap<Function *, Function *>
+MergeFunctionsPass::runOnFunctions(ArrayRef<Function *> F) {
   MergeFunctions MF;
   return MF.runOnFunctions(F);
 }
@@ -455,7 +454,7 @@ template <typename FuncContainer> bool MergeFunctions::run(FuncContainer &M) {
     // If the hash value matches the previous value or the next one, we must
     // consider merging it. Otherwise it is dropped and never considered again.
     if ((I != S && std::prev(I)->first == I->first) ||
-        (std::next(I) != IE && std::next(I)->first == I->first) ) {
+        (std::next(I) != IE && std::next(I)->first == I->first)) {
       Deferred.push_back(WeakTrackingVH(I->second));
     }
   }
@@ -489,11 +488,12 @@ template <typename FuncContainer> bool MergeFunctions::run(FuncContainer &M) {
   return Changed;
 }
 
-std::pair<bool, DenseMap<Function *, Function *>>
-MergeFunctions::runOnFunctions(std::set<Function *> &F) {
-  bool MergeResult = false;
-  MergeResult = this->run(F);
-  return {MergeResult, this->DelToNewMap};
+DenseMap<Function *, Function *>
+MergeFunctions::runOnFunctions(ArrayRef<Function *> F) {
+  bool MergeResult = this->run(F);
+  if (!MergeResult)
+    this->DelToNewMap = DenseMap<Function *, Function *>();
+  return this->DelToNewMap;
 }
 
 // Replace direct callers of Old with New.
diff --git a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
index a58ce6f41ec404..42c58accce51de 100644
--- a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
@@ -7,6 +7,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/IPO/MergeFunctions.h"
+
+#include "llvm/ADT/SetVector.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
@@ -116,22 +118,18 @@ TEST(MergeFunctions, TrueOutputFunctionsTest) {
     )invalid",
                                                 Err, Ctx));
 
-  std::set<Function *> FunctionsSet;
+  SetVector<Function *> FunctionsSet;
   for (Function &F : *M)
     FunctionsSet.insert(&F);
 
-  std::pair<bool, DenseMap<Function *, Function *>> MergeResult =
-      MergeFunctionsPass::runOnFunctions(FunctionsSet);
-
-  // Expects true after merging _slice_add10 and _slice_add10_alt
-  EXPECT_TRUE(MergeResult.first);
+  DenseMap<Function *, Function *> MergeResult =
+      MergeFunctionsPass::runOnFunctions(FunctionsSet.getArrayRef());
 
   // Expects that both functions (_slice_add10 and _slice_add10_alt)
   // be mapped to the same new function
-  EXPECT_TRUE(MergeResult.second.size() > 0);
-  DenseMap<Function *, Function *> DelToNew = MergeResult.second;
+  EXPECT_TRUE(!MergeResult.empty());
   Function *NewFunction = M->getFunction("_slice_add10");
-  for (auto P : DelToNew)
+  for (auto P : MergeResult)
     if (P.second)
       EXPECT_EQ(P.second, NewFunction);
 }
@@ -234,21 +232,18 @@ TEST(MergeFunctions, FalseOutputFunctionsTest) {
     )invalid",
                                                 Err, Ctx));
 
-  std::set<Function *> FunctionsSet;
+  SetVector<Function *> FunctionsSet;
   for (Function &F : *M)
     FunctionsSet.insert(&F);
 
-  std::pair<bool, DenseMap<Function *, Function *>> MergeResult =
-      MergeFunctionsPass::runOnFunctions(FunctionsSet);
+  DenseMap<Function *, Function *> MergeResult =
+      MergeFunctionsPass::runOnFunctions(FunctionsSet.getArrayRef());
 
-  for (auto P : MergeResult.second)
+  for (auto P : MergeResult)
     std::cout << P.first << " " << P.second << "\n";
 
-  // Expects false after trying to merge _slice_add10 and _slice_add10_alt
-  EXPECT_FALSE(MergeResult.first);
-
   // Expects empty map
-  EXPECT_EQ(MergeResult.second.size(), 0u);
+  EXPECT_EQ(MergeResult.size(), 0u);
 }
 
 } // namespace
\ No newline at end of file

>From 07aa7c1d29fafab9b89401349f20ccb9007b5733 Mon Sep 17 00:00:00 2001
From: Casperento <44746868+Casperento at users.noreply.github.com>
Date: Wed, 27 Nov 2024 17:25:16 -0300
Subject: [PATCH 09/11] comment fix

---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index ea013245b50148..1197d65e99eef6 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -491,8 +491,7 @@ template <typename FuncContainer> bool MergeFunctions::run(FuncContainer &M) {
 DenseMap<Function *, Function *>
 MergeFunctions::runOnFunctions(ArrayRef<Function *> F) {
   bool MergeResult = this->run(F);
-  if (!MergeResult)
-    this->DelToNewMap = DenseMap<Function *, Function *>();
+  assert(MergeResult == !DelToNewMap.empty());
   return this->DelToNewMap;
 }
 

>From a6f6b3c141e7ee5970435c62a66164d44c4287ee Mon Sep 17 00:00:00 2001
From: Rafael Eckstein <44746868+Casperento at users.noreply.github.com>
Date: Thu, 28 Nov 2024 07:45:25 -0300
Subject: [PATCH 10/11] comment fix

Co-authored-by: Nikita Popov <github at npopov.com>
---
 llvm/lib/Transforms/IPO/MergeFunctions.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
index 1197d65e99eef6..e8508416f54275 100644
--- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -490,7 +490,7 @@ template <typename FuncContainer> bool MergeFunctions::run(FuncContainer &M) {
 
 DenseMap<Function *, Function *>
 MergeFunctions::runOnFunctions(ArrayRef<Function *> F) {
-  bool MergeResult = this->run(F);
+  [[maybe_unused]] bool MergeResult = this->run(F);
   assert(MergeResult == !DelToNewMap.empty());
   return this->DelToNewMap;
 }

>From 87032639161ed139e906ee76b9bdb3184d44d26f Mon Sep 17 00:00:00 2001
From: Casperento <44746868+Casperento at users.noreply.github.com>
Date: Thu, 28 Nov 2024 10:46:21 -0300
Subject: [PATCH 11/11] comment fix

---
 llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
index 42c58accce51de..56d119878a9ab2 100644
--- a/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/MergeFunctionsTest.cpp
@@ -239,9 +239,6 @@ TEST(MergeFunctions, FalseOutputFunctionsTest) {
   DenseMap<Function *, Function *> MergeResult =
       MergeFunctionsPass::runOnFunctions(FunctionsSet.getArrayRef());
 
-  for (auto P : MergeResult)
-    std::cout << P.first << " " << P.second << "\n";
-
   // Expects empty map
   EXPECT_EQ(MergeResult.size(), 0u);
 }



More information about the llvm-commits mailing list