Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CGData] Outlined Hash Tree #89792

Merged
merged 3 commits into from
Jul 7, 2024
Merged

Conversation

kyulee-com
Copy link
Contributor

@kyulee-com kyulee-com commented Apr 23, 2024

This defines the OutlinedHashTree class.
It contains sequences of stable hash values of instructions that have been outlined. This OutlinedHashTree can be used to track the outlined instruction sequences across modules. A trie structure is used in its implementation, allowing for a compact sharing of common prefixes.

This is a patch for https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.

@kyulee-com kyulee-com force-pushed the outlinedhashtree branch 3 times, most recently from e4d31d4 to e405e02 Compare April 23, 2024 21:24
@kyulee-com kyulee-com changed the title [CGData][OutlinedHashTree] Define OutlinedHashTree [CGData] OutlinedHashTree Apr 25, 2024
Copy link
Contributor

@xuanzhang816 xuanzhang816 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finished reviewing code related to OutlinedHashTree.

llvm/include/llvm/CodeGenData/OutlinedHashTree.h Outdated Show resolved Hide resolved
llvm/include/llvm/CodeGenData/OutlinedHashTree.h Outdated Show resolved Hide resolved
llvm/include/llvm/CodeGenData/OutlinedHashTree.h Outdated Show resolved Hide resolved
llvm/lib/CodeGenData/OutlinedHashTree.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGenData/OutlinedHashTree.cpp Show resolved Hide resolved
llvm/lib/CodeGenData/OutlinedHashTree.cpp Show resolved Hide resolved
@kyulee-com
Copy link
Contributor Author

@xuanzh-meta Thanks for the review! I incorporated the fixes.

@kyulee-com kyulee-com changed the title [CGData] OutlinedHashTree [CGData] Outlined Hash Tree May 3, 2024
llvm/lib/CodeGenData/OutlinedHashTreeRecord.cpp Outdated Show resolved Hide resolved
llvm/include/llvm/CodeGenData/OutlinedHashTree.h Outdated Show resolved Hide resolved
llvm/include/llvm/CodeGenData/OutlinedHashTree.h Outdated Show resolved Hide resolved
llvm/include/llvm/CodeGenData/OutlinedHashTree.h Outdated Show resolved Hide resolved
llvm/lib/CodeGenData/OutlinedHashTree.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGenData/OutlinedHashTree.cpp Outdated Show resolved Hide resolved
llvm/lib/Support/SuffixTree.cpp Outdated Show resolved Hide resolved
llvm/lib/Support/SuffixTree.cpp Outdated Show resolved Hide resolved
llvm/lib/Support/SuffixTree.cpp Outdated Show resolved Hide resolved
@kyulee-com
Copy link
Contributor Author

@ellishg Thanks for the review! I incorporated the fixes.

@llvmbot
Copy link
Collaborator

llvmbot commented May 5, 2024

@llvm/pr-subscribers-backend-arm
@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-llvm-support

Author: Kyungwoo Lee (kyulee-com)

Changes

This defines the OutlinedHashTree class.
It contains sequences of stable hash values of instructions that have been outlined. This OutlinedHashTree can be used to track the outlined instruction sequences across modules. A trie structure is used in its implementation, allowing for a compact sharing of common prefixes.

This is a patch for https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.


Patch is 89.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/89792.diff

30 Files Affected:

  • (added) llvm/include/llvm/CodeGenData/OutlinedHashTree.h (+98)
  • (added) llvm/include/llvm/CodeGenData/OutlinedHashTreeRecord.h (+76)
  • (modified) llvm/include/llvm/Support/SuffixTree.h (+31-3)
  • (modified) llvm/include/llvm/Support/SuffixTreeNode.h (+24-1)
  • (modified) llvm/lib/CMakeLists.txt (+1)
  • (modified) llvm/lib/CodeGen/MachineOutliner.cpp (+23-13)
  • (added) llvm/lib/CodeGenData/CMakeLists.txt (+14)
  • (added) llvm/lib/CodeGenData/OutlinedHashTree.cpp (+132)
  • (added) llvm/lib/CodeGenData/OutlinedHashTreeRecord.cpp (+168)
  • (modified) llvm/lib/Support/SuffixTree.cpp (+81-2)
  • (modified) llvm/lib/Support/SuffixTreeNode.cpp (+5)
  • (modified) llvm/test/CodeGen/AArch64/machine-outliner-cfi-tail-some.mir (+1-1)
  • (added) llvm/test/CodeGen/AArch64/machine-outliner-leaf-descendants.ll (+124)
  • (modified) llvm/test/CodeGen/AArch64/machine-outliner-overlap.mir (+29-7)
  • (modified) llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.mir (+1-1)
  • (modified) llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-thunk.ll (+2-2)
  • (added) llvm/test/CodeGen/AArch64/machine-outliner-sort-per-priority.ll (+96)
  • (modified) llvm/test/CodeGen/AArch64/machine-outliner-throw2.ll (+2-2)
  • (modified) llvm/test/CodeGen/AArch64/machine-outliner-thunk.ll (+1-1)
  • (modified) llvm/test/CodeGen/AArch64/machine-outliner.mir (+1-1)
  • (modified) llvm/test/CodeGen/ARM/machine-outliner-calls.mir (+28-52)
  • (modified) llvm/test/CodeGen/ARM/machine-outliner-default.mir (+15-18)
  • (modified) llvm/test/CodeGen/ARM/machine-outliner-stack-fixup-arm.mir (+24-32)
  • (modified) llvm/test/CodeGen/ARM/machine-outliner-stack-fixup-thumb.mir (+27-47)
  • (modified) llvm/test/CodeGen/RISCV/machineoutliner-pcrel-lo.mir (+4-4)
  • (modified) llvm/unittests/CMakeLists.txt (+1)
  • (added) llvm/unittests/CodeGenData/CMakeLists.txt (+14)
  • (added) llvm/unittests/CodeGenData/OutlinedHashTreeRecordTest.cpp (+118)
  • (added) llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp (+82)
  • (modified) llvm/unittests/Support/SuffixTreeTest.cpp (+134)
diff --git a/llvm/include/llvm/CodeGenData/OutlinedHashTree.h b/llvm/include/llvm/CodeGenData/OutlinedHashTree.h
new file mode 100644
index 00000000000000..c40038cd8c5174
--- /dev/null
+++ b/llvm/include/llvm/CodeGenData/OutlinedHashTree.h
@@ -0,0 +1,98 @@
+//===- OutlinedHashTree.h --------------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===---------------------------------------------------------------------===//
+//
+// This defines the OutlinedHashTree class. It contains sequences of stable
+// hash values of instructions that have been outlined. This OutlinedHashTree
+// can be used to track the outlined instruction sequences across modules.
+//
+//===---------------------------------------------------------------------===//
+
+#ifndef LLVM_CODEGENDATA_OUTLINEDHASHTREE_H
+#define LLVM_CODEGENDATA_OUTLINEDHASHTREE_H
+
+#include "llvm/ADT/StableHashing.h"
+#include "llvm/ObjectYAML/YAML.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include <unordered_map>
+#include <vector>
+
+namespace llvm {
+
+/// A HashNode is an entry in an OutlinedHashTree, holding a hash value
+/// and a collection of Successors (other HashNodes). If a HashNode has
+/// a positive terminal value (Terminals > 0), it signifies the end of
+/// a hash sequence with that occurrence count.
+struct HashNode {
+  /// The hash value of the node.
+  stable_hash Hash = 0;
+  /// The number of terminals in the sequence ending at this node.
+  std::optional<unsigned> Terminals;
+  /// The successors of this node.
+  /// We don't use DenseMap as a stable_hash value can be tombstone.
+  std::unordered_map<stable_hash, std::unique_ptr<HashNode>> Successors;
+};
+
+class OutlinedHashTree {
+
+  using EdgeCallbackFn =
+      std::function<void(const HashNode *, const HashNode *)>;
+  using NodeCallbackFn = std::function<void(const HashNode *)>;
+
+  using HashSequence = SmallVector<stable_hash>;
+  using HashSequencePair = std::pair<HashSequence, unsigned>;
+
+public:
+  /// Walks every edge and node in the OutlinedHashTree and calls CallbackEdge
+  /// for the edges and CallbackNode for the nodes with the stable_hash for
+  /// the source and the stable_hash of the sink for an edge. These generic
+  /// callbacks can be used to traverse a OutlinedHashTree for the purpose of
+  /// print debugging or serializing it.
+  void walkGraph(NodeCallbackFn CallbackNode,
+                 EdgeCallbackFn CallbackEdge = nullptr,
+                 bool SortedWalk = false) const;
+
+  /// Release all hash nodes except the root hash node.
+  void clear() {
+    assert(getRoot()->Hash == 0 && !getRoot()->Terminals);
+    getRoot()->Successors.clear();
+  }
+
+  /// \returns true if the hash tree has only the root node.
+  bool empty() { return size() == 1; }
+
+  /// \returns the size of a OutlinedHashTree by traversing it. If
+  /// \p GetTerminalCountOnly is true, it only counts the terminal nodes
+  /// (meaning it returns the the number of hash sequences in the
+  /// OutlinedHashTree).
+  size_t size(bool GetTerminalCountOnly = false) const;
+
+  /// \returns the depth of a OutlinedHashTree by traversing it.
+  size_t depth() const;
+
+  /// \returns the root hash node of a OutlinedHashTree.
+  const HashNode *getRoot() const { return &Root; }
+  HashNode *getRoot() { return &Root; }
+
+  /// Inserts a \p Sequence into the this tree. The last node in the sequence
+  /// will increase Terminals.
+  void insert(const HashSequencePair &SequencePair);
+
+  /// Merge a \p OtherTree into this Tree.
+  void merge(const OutlinedHashTree *OtherTree);
+
+  /// \returns the matching count if \p Sequence exists in the OutlinedHashTree.
+  std::optional<unsigned> find(const HashSequence &Sequence) const;
+
+private:
+  HashNode Root;
+};
+
+} // namespace llvm
+
+#endif
diff --git a/llvm/include/llvm/CodeGenData/OutlinedHashTreeRecord.h b/llvm/include/llvm/CodeGenData/OutlinedHashTreeRecord.h
new file mode 100644
index 00000000000000..2960e319604489
--- /dev/null
+++ b/llvm/include/llvm/CodeGenData/OutlinedHashTreeRecord.h
@@ -0,0 +1,76 @@
+//===- OutlinedHashTreeRecord.h --------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===---------------------------------------------------------------------===//
+//
+// This defines the OutlinedHashTreeRecord class. This class holds the outlined
+// hash tree for both serialization and deserialization processes. It utilizes
+// two data formats for serialization: raw binary data and YAML.
+// These two formats can be used interchangeably.
+//
+//===---------------------------------------------------------------------===//
+
+#ifndef LLVM_CODEGENDATA_OUTLINEDHASHTREERECORD_H
+#define LLVM_CODEGENDATA_OUTLINEDHASHTREERECORD_H
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/CodeGenData/OutlinedHashTree.h"
+
+namespace llvm {
+
+/// HashNodeStable is the serialized, stable, and compact representation
+/// of a HashNode.
+struct HashNodeStable {
+  llvm::yaml::Hex64 Hash;
+  unsigned Terminals;
+  std::vector<unsigned> SuccessorIds;
+};
+
+using IdHashNodeStableMapTy = std::map<unsigned, HashNodeStable>;
+using IdHashNodeMapTy = DenseMap<unsigned, HashNode *>;
+using HashNodeIdMapTy = DenseMap<const HashNode *, unsigned>;
+
+struct OutlinedHashTreeRecord {
+  std::unique_ptr<OutlinedHashTree> HashTree;
+
+  OutlinedHashTreeRecord() { HashTree = std::make_unique<OutlinedHashTree>(); }
+  OutlinedHashTreeRecord(std::unique_ptr<OutlinedHashTree> HashTree)
+      : HashTree(std::move(HashTree)){};
+
+  /// Serialize the outlined hash tree to a raw_ostream.
+  void serialize(raw_ostream &OS) const;
+  /// Deserialize the outlined hash tree from a raw_ostream.
+  void deserialize(const unsigned char *&Ptr);
+  /// Serialize the outlined hash tree to a YAML stream.
+  void serializeYAML(yaml::Output &YOS) const;
+  /// Deserialize the outlined hash tree from a YAML stream.
+  void deserializeYAML(yaml::Input &YIS);
+
+  /// Merge the other outlined hash tree into this one.
+  void merge(const OutlinedHashTreeRecord &Other) {
+    HashTree->merge(Other.HashTree.get());
+  }
+
+  /// \returns true if the outlined hash tree is empty.
+  bool empty() const { return HashTree->empty(); }
+
+  /// Print the outlined hash tree in a YAML format.
+  void print(raw_ostream &OS = llvm::errs()) const {
+    yaml::Output YOS(OS);
+    serializeYAML(YOS);
+  }
+
+private:
+  /// Convert the outlined hash tree to stable data.
+  void convertToStableData(IdHashNodeStableMapTy &IdNodeStableMap) const;
+
+  /// Convert the stable data back to the outlined hash tree.
+  void convertFromStableData(const IdHashNodeStableMapTy &IdNodeStableMap);
+};
+
+} // end namespace llvm
+
+#endif // LLVM_CODEGENDATA_OUTLINEDHASHTREERECORD_H
diff --git a/llvm/include/llvm/Support/SuffixTree.h b/llvm/include/llvm/Support/SuffixTree.h
index 4940fbbf308d8b..37b73666404300 100644
--- a/llvm/include/llvm/Support/SuffixTree.h
+++ b/llvm/include/llvm/Support/SuffixTree.h
@@ -42,6 +42,9 @@ class SuffixTree {
   /// Each element is an integer representing an instruction in the module.
   ArrayRef<unsigned> Str;
 
+  /// Whether to consider leaf descendants or only leaf children.
+  bool OutlinerLeafDescendants;
+
   /// A repeated substring in the tree.
   struct RepeatedSubstring {
     /// The length of the string.
@@ -130,11 +133,27 @@ class SuffixTree {
   /// this step.
   unsigned extend(unsigned EndIdx, unsigned SuffixesToAdd);
 
+  /// This vector contains all leaf nodes of this suffix tree. These leaf nodes
+  /// are identified using post-order depth-first traversal, so that the order
+  /// of these leaf nodes in the vector matches the order of the leaves in the
+  /// tree from left to right if one were to draw the tree on paper.
+  std::vector<SuffixTreeLeafNode *> LeafNodes;
+
+  /// Perform a post-order depth-first traversal of the tree and perform two
+  /// tasks during the traversal. The first is to populate LeafNodes, adding
+  /// nodes in order of the traversal. The second is to keep track of the leaf
+  /// descendants of every internal node by assigning values to LeftLeafIndex
+  /// and RightLefIndex fields of SuffixTreeNode for all internal nodes.
+  void setLeafNodes();
+
 public:
   /// Construct a suffix tree from a sequence of unsigned integers.
   ///
   /// \param Str The string to construct the suffix tree for.
-  SuffixTree(const ArrayRef<unsigned> &Str);
+  /// \param OutlinerLeafDescendants Whether to consider leaf descendants or
+  /// only leaf children (used by Machine Outliner).
+  SuffixTree(const ArrayRef<unsigned> &Str,
+             bool OutlinerLeafDescendants = false);
 
   /// Iterator for finding all repeated substrings in the suffix tree.
   struct RepeatedSubstringIterator {
@@ -154,6 +173,12 @@ class SuffixTree {
     /// instruction lengths.
     const unsigned MinLength = 2;
 
+    /// Vector of leaf nodes of the suffix tree.
+    const std::vector<SuffixTreeLeafNode *> &LeafNodes;
+
+    /// Whether to consider leaf descendants or only leaf children.
+    bool OutlinerLeafDescendants = !LeafNodes.empty();
+
     /// Move the iterator to the next repeated substring.
     void advance();
 
@@ -179,7 +204,10 @@ class SuffixTree {
       return !(*this == Other);
     }
 
-    RepeatedSubstringIterator(SuffixTreeInternalNode *N) : N(N) {
+    RepeatedSubstringIterator(
+        SuffixTreeInternalNode *N,
+        const std::vector<SuffixTreeLeafNode *> &LeafNodes = {})
+        : N(N), LeafNodes(LeafNodes) {
       // Do we have a non-null node?
       if (!N)
         return;
@@ -191,7 +219,7 @@ class SuffixTree {
   };
 
   typedef RepeatedSubstringIterator iterator;
-  iterator begin() { return iterator(Root); }
+  iterator begin() { return iterator(Root, LeafNodes); }
   iterator end() { return iterator(nullptr); }
 };
 
diff --git a/llvm/include/llvm/Support/SuffixTreeNode.h b/llvm/include/llvm/Support/SuffixTreeNode.h
index 7d0d1cf0c58b95..84b590f2deb0cd 100644
--- a/llvm/include/llvm/Support/SuffixTreeNode.h
+++ b/llvm/include/llvm/Support/SuffixTreeNode.h
@@ -46,6 +46,17 @@ struct SuffixTreeNode {
   /// the root to this node.
   unsigned ConcatLen = 0;
 
+  /// These two indices give a range of indices for its leaf descendants.
+  /// Imagine drawing a tree on paper and assigning a unique index to each leaf
+  /// node in monotonically increasing order from left to right. This way of
+  /// numbering the leaf nodes allows us to associate a continuous range of
+  /// indices with each internal node. For example, if a node has leaf
+  /// descendants with indices i, i+1, ..., j, then its LeftLeafIdx is i and
+  /// its RightLeafIdx is j. These indices are for LeafNodes in the SuffixTree
+  /// class, which is constructed using post-order depth-first traversal.
+  unsigned LeftLeafIdx = EmptyIdx;
+  unsigned RightLeafIdx = EmptyIdx;
+
 public:
   // LLVM RTTI boilerplate.
   NodeKind getKind() const { return Kind; }
@@ -56,6 +67,18 @@ struct SuffixTreeNode {
   /// \returns the end index of this node.
   virtual unsigned getEndIdx() const = 0;
 
+  /// \return the index of this node's left most leaf node.
+  unsigned getLeftLeafIdx() const;
+
+  /// \return the index of this node's right most leaf node.
+  unsigned getRightLeafIdx() const;
+
+  /// Set the index of the left most leaf node of this node to \p Idx.
+  void setLeftLeafIdx(unsigned Idx);
+
+  /// Set the index of the right most leaf node of this node to \p Idx.
+  void setRightLeafIdx(unsigned Idx);
+
   /// Advance this node's StartIdx by \p Inc.
   void incrementStartIdx(unsigned Inc);
 
@@ -168,4 +191,4 @@ struct SuffixTreeLeafNode : SuffixTreeNode {
   virtual ~SuffixTreeLeafNode() = default;
 };
 } // namespace llvm
-#endif // LLVM_SUPPORT_SUFFIXTREE_NODE_H
\ No newline at end of file
+#endif // LLVM_SUPPORT_SUFFIXTREE_NODE_H
diff --git a/llvm/lib/CMakeLists.txt b/llvm/lib/CMakeLists.txt
index 74e2d03c07953d..2ac0b0dc026e16 100644
--- a/llvm/lib/CMakeLists.txt
+++ b/llvm/lib/CMakeLists.txt
@@ -10,6 +10,7 @@ add_subdirectory(InterfaceStub)
 add_subdirectory(IRPrinter)
 add_subdirectory(IRReader)
 add_subdirectory(CodeGen)
+add_subdirectory(CodeGenData)
 add_subdirectory(CodeGenTypes)
 add_subdirectory(BinaryFormat)
 add_subdirectory(Bitcode)
diff --git a/llvm/lib/CodeGen/MachineOutliner.cpp b/llvm/lib/CodeGen/MachineOutliner.cpp
index dc2f5ef15206e8..01a9966adb097a 100644
--- a/llvm/lib/CodeGen/MachineOutliner.cpp
+++ b/llvm/lib/CodeGen/MachineOutliner.cpp
@@ -121,6 +121,12 @@ static cl::opt<unsigned> OutlinerBenefitThreshold(
     cl::desc(
         "The minimum size in bytes before an outlining candidate is accepted"));
 
+static cl::opt<bool> OutlinerLeafDescendants(
+    "outliner-leaf-descendants", cl::init(true), cl::Hidden,
+    cl::desc("Consider all leaf descendants of internal nodes of the suffix "
+             "tree as candidates for outlining (if false, only leaf children "
+             "are considered)"));
+
 namespace {
 
 /// Maps \p MachineInstrs to unsigned integers and stores the mappings.
@@ -576,7 +582,7 @@ void MachineOutliner::emitOutlinedFunctionRemark(OutlinedFunction &OF) {
 void MachineOutliner::findCandidates(
     InstructionMapper &Mapper, std::vector<OutlinedFunction> &FunctionList) {
   FunctionList.clear();
-  SuffixTree ST(Mapper.UnsignedVec);
+  SuffixTree ST(Mapper.UnsignedVec, OutlinerLeafDescendants);
 
   // First, find all of the repeated substrings in the tree of minimum length
   // 2.
@@ -593,7 +599,11 @@ void MachineOutliner::findCandidates(
     unsigned NumDiscarded = 0;
     unsigned NumKept = 0;
 #endif
-    for (const unsigned &StartIdx : RS.StartIndices) {
+    // Sort the start indices so that we can efficiently check if candidates
+    // overlap with each other in MachineOutliner::findCandidates().
+    SmallVector<unsigned> SortedStartIndices(RS.StartIndices);
+    llvm::sort(SortedStartIndices);
+    for (const unsigned &StartIdx : SortedStartIndices) {
       // Trick: Discard some candidates that would be incompatible with the
       // ones we've already found for this sequence. This will save us some
       // work in candidate selection.
@@ -616,17 +626,15 @@ void MachineOutliner::findCandidates(
       // * End before the other starts
       // * Start after the other ends
       unsigned EndIdx = StartIdx + StringLen - 1;
-      auto FirstOverlap = find_if(
-          CandidatesForRepeatedSeq, [StartIdx, EndIdx](const Candidate &C) {
-            return EndIdx >= C.getStartIdx() && StartIdx <= C.getEndIdx();
-          });
-      if (FirstOverlap != CandidatesForRepeatedSeq.end()) {
+      if (CandidatesForRepeatedSeq.size() > 0 &&
+          StartIdx <= CandidatesForRepeatedSeq.back().getEndIdx()) {
 #ifndef NDEBUG
         ++NumDiscarded;
-        LLVM_DEBUG(dbgs() << "    .. DISCARD candidate @ [" << StartIdx
-                          << ", " << EndIdx << "]; overlaps with candidate @ ["
-                          << FirstOverlap->getStartIdx() << ", "
-                          << FirstOverlap->getEndIdx() << "]\n");
+        LLVM_DEBUG(dbgs() << "    .. DISCARD candidate @ [" << StartIdx << ", "
+                          << EndIdx << "]; overlaps with candidate @ ["
+                          << CandidatesForRepeatedSeq.back().getStartIdx()
+                          << ", " << CandidatesForRepeatedSeq.back().getEndIdx()
+                          << "]\n");
 #endif
         continue;
       }
@@ -828,10 +836,12 @@ bool MachineOutliner::outline(Module &M,
                     << "\n");
   bool OutlinedSomething = false;
 
-  // Sort by benefit. The most beneficial functions should be outlined first.
+  // Sort by priority where priority := getNotOutlinedCost / getOutliningCost.
+  // The function with highest priority should be outlined first.
   stable_sort(FunctionList,
               [](const OutlinedFunction &LHS, const OutlinedFunction &RHS) {
-                return LHS.getBenefit() > RHS.getBenefit();
+                return LHS.getNotOutlinedCost() * RHS.getOutliningCost() >
+                       RHS.getNotOutlinedCost() * LHS.getOutliningCost();
               });
 
   // Walk over each function, outlining them as we go along. Functions are
diff --git a/llvm/lib/CodeGenData/CMakeLists.txt b/llvm/lib/CodeGenData/CMakeLists.txt
new file mode 100644
index 00000000000000..3ba90f96cc86d4
--- /dev/null
+++ b/llvm/lib/CodeGenData/CMakeLists.txt
@@ -0,0 +1,14 @@
+add_llvm_component_library(LLVMCodeGenData
+  OutlinedHashTree.cpp
+  OutlinedHashTreeRecord.cpp
+
+  ADDITIONAL_HEADER_DIRS
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/CodeGenData
+
+  DEPENDS
+  intrinsics_gen
+
+  LINK_COMPONENTS
+  Core
+  Support
+  )
diff --git a/llvm/lib/CodeGenData/OutlinedHashTree.cpp b/llvm/lib/CodeGenData/OutlinedHashTree.cpp
new file mode 100644
index 00000000000000..3111f7e910dc30
--- /dev/null
+++ b/llvm/lib/CodeGenData/OutlinedHashTree.cpp
@@ -0,0 +1,132 @@
+//===-- OutlinedHashTree.cpp ----------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// An OutlinedHashTree is a Trie that contains sequences of stable hash values
+// of instructions that have been outlined. This OutlinedHashTree can be used
+// to understand the outlined instruction sequences collected across modules.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGenData/OutlinedHashTree.h"
+
+#include <stack>
+#include <tuple>
+
+#define DEBUG_TYPE "outlined-hash-tree"
+
+using namespace llvm;
+
+void OutlinedHashTree::walkGraph(NodeCallbackFn CallbackNode,
+                                 EdgeCallbackFn CallbackEdge,
+                                 bool SortedWalk) const {
+  SmallVector<const HashNode *> Stack;
+  Stack.emplace_back(getRoot());
+
+  while (!Stack.empty()) {
+    const auto *Current = Stack.pop_back_val();
+    if (CallbackNode)
+      CallbackNode(Current);
+
+    auto HandleNext = [&](const HashNode *Next) {
+      if (CallbackEdge)
+        CallbackEdge(Current, Next);
+      Stack.emplace_back(Next);
+    };
+    if (SortedWalk) {
+      std::map<stable_hash, const HashNode *> SortedSuccessors;
+      for (const auto &P : Current->Successors)
+        SortedSuccessors[P.first] = P.second.get();
+      for (const auto &P : SortedSuccessors)
+        HandleNext(P.second);
+    } else {
+      for (const auto &P : Current->Successors)
+        HandleNext(P.second.get());
+    }
+  }
+}
+
+size_t OutlinedHashTree::size(bool GetTerminalCountOnly) const {
+  size_t Size = 0;
+  walkGraph([&Size, GetTerminalCountOnly](const HashNode *N) {
+    Size += (N && (!GetTerminalCountOnly || N->Terminals));
+  });
+  return Size;
+}
+
+size_t OutlinedHashTree::depth() const {
+  size_t Size = 0;
+  std::unordered_map<const HashNode *, size_t> DepthMap;
+  walkGraph([&Size, &DepthMap](
+                const HashNode *N) { Size = std::max(Size, DepthMap[N]); },
+            [&DepthMap](const HashNode *Src, const HashNode *Dst) {
+              size_t Depth = DepthMap[Src];
+              DepthMap[Dst] = Depth + 1;
+            });
+  return Size;
+}
+
+void OutlinedHashTree::insert(const HashSequencePair &SequencePair) {
+  const auto &Sequence = SequencePair.first;
+  unsigned Count = SequencePair.second;
+  HashNode *Current = getRoot();
+
+  for (stable_hash StableHash : Sequence) {
+    auto I = Current->Successors.find(StableHash);
+    if (I == Current->Successors.end()) {
+      std::unique_ptr<HashNode> Next = std::make_unique<HashNode>();
+      HashNode *NextPtr = Next.get();
+      NextPtr->Hash = StableHash;
+      Current->Successors.emplace(StableHash, std::move(Next));
+      Current = NextPtr;
+    } else
+      Current = I->second.get();
+  }
+  if (Count)
+    Current->Terminals = (Current->Terminals ? *Current->Terminals : 0) + Count;
+}
+
+voi...
[truncated]

@kyulee-com
Copy link
Contributor Author

To eliminate any confusion for reviewing this PR, I now removed the stacked commits related to Part 1: https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-1-fulllto-part-2-thinlto-nolto-to-come/78732

@kyulee-com kyulee-com force-pushed the outlinedhashtree branch 3 times, most recently from 5205a52 to c05bdf6 Compare May 5, 2024 14:25
@kyulee-com kyulee-com mentioned this pull request May 6, 2024
@kyulee-com kyulee-com requested a review from alx32 June 18, 2024 15:57
llvm/lib/CodeGenData/OutlinedHashTree.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGenData/OutlinedHashTree.cpp Outdated Show resolved Hide resolved
llvm/lib/CodeGenData/OutlinedHashTree.cpp Outdated Show resolved Hide resolved
llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@ellishg ellishg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -0,0 +1,14 @@
add_llvm_component_library(LLVMCodeGenData
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are you using this library? It seems to only be defined and only used in the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. It is currently used only in the test, but the subsequent PRs will use it.

This defines the OutlinedHashTree class.
It contains sequences of stable hash values of instructions that have been outlined.
This OutlinedHashTree can be used to track the outlined instruction sequences across modules.
A trie structure is used in its implementation, allowing for a compact sharing of common prefixes.
Copy link

github-actions bot commented Jul 6, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@kyulee-com kyulee-com merged commit d00f1c1 into llvm:main Jul 7, 2024
7 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 7, 2024

LLVM Buildbot has detected a new failure on builder ppc64le-lld-multistage-test running on ppc64le-lld-multistage-test while building llvm at step 12 "build-stage2-unified-tree".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/776

Here is the relevant piece of the build log for the reference:

Step 12 (build-stage2-unified-tree) failure: build (failure)
...
357.094 [163/604/5400] Building CXX object tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaDeclObjC.cpp.o
357.106 [163/603/5401] Building CXX object tools/bugpoint/CMakeFiles/bugpoint.dir/ExecutionDriver.cpp.o
357.367 [163/602/5402] Building CXX object tools/lld/COFF/CMakeFiles/lldCOFF.dir/ICF.cpp.o
357.444 [163/601/5403] Building CXX object unittests/MC/CMakeFiles/MCTests.dir/Disassembler.cpp.o
357.512 [163/600/5404] Building CXX object tools/lld/ELF/CMakeFiles/lldELF.dir/Arch/AMDGPU.cpp.o
357.533 [163/599/5405] Building CXX object tools/lld/ELF/CMakeFiles/lldELF.dir/Arch/AVR.cpp.o
357.661 [163/598/5406] Building CXX object lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/GCNRewritePartialRegUses.cpp.o
357.700 [163/597/5407] Building CXX object unittests/Passes/PassBuilderBindings/CMakeFiles/PassesBindingsTests.dir/PassBuilderBindingsTest.cpp.o
357.729 [162/597/5408] Building CXX object lib/Target/AMDGPU/MCTargetDesc/CMakeFiles/LLVMAMDGPUDesc.dir/AMDGPUELFObjectWriter.cpp.o
357.803 [162/596/5409] Building CXX object unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o
FAILED: unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o 
ccache /home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/build/stage2/unittests/CodeGenData -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/CodeGenData -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/build/stage2/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googletest/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googlemock/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -std=c++17  -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -Wno-suggest-override -MD -MT unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o -MF unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o.d -o unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o -c /home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp
In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp:10:
In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googlemock/include/gmock/gmock.h:56:
In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googlemock/include/gmock/gmock-actions.h:145:
In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googlemock/include/gmock/internal/gmock-internal-utils.h:50:
/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1379:11: error: comparison of integers of different signs: 'const unsigned long' and 'const int' [-Werror,-Wsign-compare]
 1379 |   if (lhs == rhs) {
      |       ~~~ ^  ~~~
/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1398:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned long, int>' requested here
 1398 |     return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
      |            ^
/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp:21:3: note: in instantiation of function template specialization 'testing::internal::EqHelper::Compare<unsigned long, int, nullptr>' requested here
   21 |   EXPECT_EQ(HashTree.size(), 1);
      |   ^
/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1869:54: note: expanded from macro 'EXPECT_EQ'
 1869 |   EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
      |                                                      ^
/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1379:11: error: comparison of integers of different signs: 'const unsigned int' and 'const int' [-Werror,-Wsign-compare]
 1379 |   if (lhs == rhs) {
      |       ~~~ ^  ~~~
/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1398:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned int, int>' requested here
 1398 |     return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
      |            ^
/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp:54:3: note: in instantiation of function template specialization 'testing::internal::EqHelper::Compare<unsigned int, int, nullptr>' requested here
   54 |   EXPECT_EQ(HashTree.find({1, 2, 3}).value(), 3);
      |   ^
/home/buildbots/llvm-external-buildbots/workers/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1869:54: note: expanded from macro 'EXPECT_EQ'
 1869 |   EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
      |                                                      ^
2 errors generated.
357.900 [162/595/5410] Building CXX object lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVISelLowering.cpp.o
357.902 [162/594/5411] Building CXX object tools/lld/unittests/AsLibELF/CMakeFiles/LLDAsLibELFTests.dir/SomeDrivers.cpp.o
357.904 [162/593/5412] Building CXX object tools/llvm-mca/CMakeFiles/llvm-mca.dir/Views/RetireControlUnitStatistics.cpp.o
357.921 [162/592/5413] Building CXX object lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/AMDGPULowerKernelAttributes.cpp.o
358.225 [162/591/5414] Building CXX object tools/llvm-mca/CMakeFiles/llvm-mca.dir/Views/SummaryView.cpp.o
358.237 [162/590/5415] Building CXX object tools/llvm-dwarfdump/CMakeFiles/llvm-dwarfdump.dir/SectionSizes.cpp.o
358.245 [162/589/5416] Building CXX object tools/lld/ELF/CMakeFiles/lldELF.dir/Arch/MSP430.cpp.o
358.381 [162/588/5417] Building CXX object lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/GCNRegPressure.cpp.o

@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 7, 2024

LLVM Buildbot has detected a new failure on builder clang-ppc64le-rhel running on ppc64le-clang-rhel-test while building llvm at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/536

Here is the relevant piece of the build log for the reference:

Step 7 (test-build-unified-tree-check-all) failure: test (failure)
...
22.404 [5/3/1156] Performing build step for 'runtimes'
0.163 [2/2/1] Building CXX object compiler-rt/lib/ubsan/CMakeFiles/RTUbsan_dynamic_version_script_dummy.powerpc64le.dir/dummy.cpp.o
0.164 [1/2/2] Building CXX object compiler-rt/lib/asan/CMakeFiles/RTAsan_dynamic_version_script_dummy.powerpc64le.dir/dummy.cpp.o
0.354 [0/2/3] Linking CXX shared library /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/lib/clang/19/lib/powerpc64le-unknown-linux-gnu/libclang_rt.ubsan_standalone.so
0.383 [0/1/4] Linking CXX shared library /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/lib/clang/19/lib/powerpc64le-unknown-linux-gnu/libclang_rt.asan.so
22.801 [4/3/1157] cd /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/runtimes/runtimes-bins && /home/buildbots/llvm-external-buildbots/cmake-3.28.2/bin/cmake --build /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/runtimes/runtimes-bins/ --target runtimes-test-depends --config Release
ninja: no work to do.
22.861 [3/3/1158] No install step for 'runtimes'
22.879 [2/3/1160] Completed 'runtimes'
24.446 [2/2/1161] Building CXX object unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o
FAILED: unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o 
ccache /home/docker/llvm-external-buildbots/clang.17.0.6/bin/clang++ --gcc-toolchain=/gcc-toolchain/usr -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/unittests/CodeGenData -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/unittests/CodeGenData -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googletest/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googlemock/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -std=c++17  -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -Wno-suggest-override -MD -MT unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o -MF unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o.d -o unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o -c /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp
In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp:10:
In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googlemock/include/gmock/gmock.h:56:
In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googlemock/include/gmock/gmock-actions.h:145:
In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googlemock/include/gmock/internal/gmock-internal-utils.h:50:
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1379:11: error: comparison of integers of different signs: 'const unsigned long' and 'const int' [-Werror,-Wsign-compare]
 1379 |   if (lhs == rhs) {
      |       ~~~ ^  ~~~
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1398:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned long, int>' requested here
 1398 |     return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
      |            ^
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp:21:3: note: in instantiation of function template specialization 'testing::internal::EqHelper::Compare<unsigned long, int, nullptr>' requested here
   21 |   EXPECT_EQ(HashTree.size(), 1);
      |   ^
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1869:54: note: expanded from macro 'EXPECT_EQ'
 1869 |   EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
      |                                                      ^
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1379:11: error: comparison of integers of different signs: 'const unsigned int' and 'const int' [-Werror,-Wsign-compare]
 1379 |   if (lhs == rhs) {
      |       ~~~ ^  ~~~
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1398:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned int, int>' requested here
 1398 |     return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
      |            ^
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp:54:3: note: in instantiation of function template specialization 'testing::internal::EqHelper::Compare<unsigned int, int, nullptr>' requested here
   54 |   EXPECT_EQ(HashTree.find({1, 2, 3}).value(), 3);
      |   ^
/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1869:54: note: expanded from macro 'EXPECT_EQ'
 1869 |   EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
      |                                                      ^
2 errors generated.
24.643 [2/1/1162] Building CXX object unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeRecordTest.cpp.o
ninja: build stopped: subcommand failed.

@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 7, 2024

LLVM Buildbot has detected a new failure on builder clang-ppc64-aix running on aix-ppc64 while building llvm at step 3 "clean-build-dir".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/228

Here is the relevant piece of the build log for the reference:

Step 3 (clean-build-dir) failure: Delete failed. (failure)
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
...
524.888 [489/50/459] Building CXX object unittests/Analysis/CMakeFiles/AnalysisTests.dir/MemoryProfileInfoTest.cpp.o
527.355 [488/50/460] Building CXX object unittests/Analysis/CMakeFiles/AnalysisTests.dir/UnrollAnalyzerTest.cpp.o
527.523 [487/50/461] Building CXX object unittests/DebugInfo/DWARF/CMakeFiles/DebugInfoDWARFTests.dir/DwarfUtils.cpp.o
529.541 [486/50/462] Building CXX object unittests/Analysis/CMakeFiles/AnalysisTests.dir/FunctionPropertiesAnalysisTest.cpp.o
529.697 [485/50/463] Building CXX object unittests/Analysis/CMakeFiles/AnalysisTests.dir/ValueLatticeTest.cpp.o
529.954 [484/50/464] Building CXX object unittests/Analysis/CMakeFiles/AnalysisTests.dir/IRSimilarityIdentifierTest.cpp.o
530.972 [483/50/465] Building CXX object unittests/DebugInfo/CodeView/CMakeFiles/DebugInfoCodeViewTests.dir/GUIDFormatTest.cpp.o
531.217 [482/50/466] Building CXX object unittests/Analysis/CMakeFiles/AnalysisTests.dir/PluginInlineOrderAnalysisTest.cpp.o
531.900 [481/50/467] Building CXX object unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeRecordTest.cpp.o
532.175 [480/50/468] Building CXX object unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o
FAILED: unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o 
/usr/local/clang-17.0.2/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_LARGE_FILE_API -D_XOPEN_SOURCE=700 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/unittests/CodeGenData -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/unittests/CodeGenData -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/build/include -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/include -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googletest/include -I/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googlemock/include -mcmodel=large -fPIC -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -std=c++17  -Wno-variadic-macros -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -Wno-suggest-override -MD -MT unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o -MF unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o.d -o unittests/CodeGenData/CMakeFiles/CodeGenDataTests.dir/OutlinedHashTreeTest.cpp.o -c /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp
In file included from /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp:10:
In file included from /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googlemock/include/gmock/gmock.h:56:
In file included from /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googlemock/include/gmock/gmock-actions.h:145:
In file included from /home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googlemock/include/gmock/internal/gmock-internal-utils.h:50:
/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1379:11: error: comparison of integers of different signs: 'const unsigned long' and 'const int' [-Werror,-Wsign-compare]
 1379 |   if (lhs == rhs) {
      |       ~~~ ^  ~~~
/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1398:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned long, int>' requested here
 1398 |     return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
      |            ^
/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp:21:3: note: in instantiation of function template specialization 'testing::internal::EqHelper::Compare<unsigned long, int, nullptr>' requested here
   21 |   EXPECT_EQ(HashTree.size(), 1);
      |   ^
/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1869:54: note: expanded from macro 'EXPECT_EQ'
 1869 |   EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
      |                                                      ^
/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1379:11: error: comparison of integers of different signs: 'const unsigned int' and 'const int' [-Werror,-Wsign-compare]
 1379 |   if (lhs == rhs) {
      |       ~~~ ^  ~~~
/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1398:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned int, int>' requested here
 1398 |     return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
      |            ^
/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp:54:3: note: in instantiation of function template specialization 'testing::internal::EqHelper::Compare<unsigned int, int, nullptr>' requested here
   54 |   EXPECT_EQ(HashTree.find({1, 2, 3}).value(), 3);
      |   ^
/home/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/third-party/unittest/googletest/include/gtest/gtest.h:1869:54: note: expanded from macro 'EXPECT_EQ'
 1869 |   EXPECT_PRED_FORMAT2(::testing::internal::EqHelper::Compare, val1, val2)
      |                                                      ^
2 errors generated.
532.816 [480/49/469] Building CXX object unittests/DebugInfo/CodeView/CMakeFiles/DebugInfoCodeViewTests.dir/TypeHashingTest.cpp.o
533.295 [480/48/470] Building CXX object unittests/Analysis/CMakeFiles/AnalysisTests.dir/ReplaceWithVecLibTest.cpp.o
533.487 [480/47/471] Building CXX object unittests/Analysis/CMakeFiles/AnalysisTests.dir/PluginInlineAdvisorAnalysisTest.cpp.o
535.673 [480/46/472] Building CXX object unittests/CodeGen/GlobalISel/CMakeFiles/GlobalISelTests.dir/CallLowering.cpp.o
535.727 [480/45/473] Linking CXX executable tools/clang/unittests/Format/FormatTests
535.738 [480/44/474] Building CXX object unittests/Analysis/CMakeFiles/AnalysisTests.dir/ScalarEvolutionTest.cpp.o
537.457 [480/43/475] Building CXX object unittests/Analysis/CMakeFiles/AnalysisTests.dir/MemorySSATest.cpp.o
537.469 [480/42/476] Building CXX object unittests/DebugInfo/DWARF/CMakeFiles/DebugInfoDWARFTests.dir/DWARFLocationExpressionTest.cpp.o

[&NodeIdMap](const HashNode *Current) {
size_t Index = NodeIdMap.size();
NodeIdMap[Current] = Index;
assert(Index = NodeIdMap.size() + 1 &&
Copy link
Contributor

@GuillaumeBelz GuillaumeBelz Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kyulee-com This line looks suspicious and generates a warning:

llvm/lib/CodeGenData/OutlinedHashTreeRecord.cpp:122:22: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
  122 |         assert(Index = NodeIdMap.size() + 1 &&
      |                ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
  123 |                        "Expected size of NodeMap to increment by 1");
      |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Do you want I create an issue for that or you'll fix that in your WIP?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this. I will fix it soon!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

kyulee-com added a commit to kyulee-com/llvm-project that referenced this pull request Jul 10, 2024
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
kyulee-com added a commit that referenced this pull request Jul 23, 2024
The llvm-cgdata tool has been introduced to handle reading and writing
of codegen data. This data includes an optimistic codegen summary that
can be utilized to enhance subsequent codegen. Currently, the tool
supports saving and restoring the outlined hash tree, facilitating
machine function outlining across modules. Additional codegen summaries
can be incorporated into separate sections as required. This patch
primarily establishes basic support for the reader and writer, similar
to llvm-profdata.

The high-level operations of llvm-cgdata are as follows:
1. It reads local raw codegen data from a custom section (for example,
__llvm_outline) embedded in native binary files
2. It merges local raw codegen data into an indexed codegen data,
complete with a suitable header.
3. It handles reading and writing of the indexed codegen data into a
standalone file.

This depends on #89792.
This is a patch for
https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.

---------

Co-authored-by: Kyungwoo Lee <kyulee@fb.com>
sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024
The llvm-cgdata tool has been introduced to handle reading and writing
of codegen data. This data includes an optimistic codegen summary that
can be utilized to enhance subsequent codegen. Currently, the tool
supports saving and restoring the outlined hash tree, facilitating
machine function outlining across modules. Additional codegen summaries
can be incorporated into separate sections as required. This patch
primarily establishes basic support for the reader and writer, similar
to llvm-profdata.

The high-level operations of llvm-cgdata are as follows:
1. It reads local raw codegen data from a custom section (for example,
__llvm_outline) embedded in native binary files
2. It merges local raw codegen data into an indexed codegen data,
complete with a suitable header.
3. It handles reading and writing of the indexed codegen data into a
standalone file.

This depends on llvm#89792.
This is a patch for
https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.

---------

Co-authored-by: Kyungwoo Lee <kyulee@fb.com>
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
The llvm-cgdata tool has been introduced to handle reading and writing
of codegen data. This data includes an optimistic codegen summary that
can be utilized to enhance subsequent codegen. Currently, the tool
supports saving and restoring the outlined hash tree, facilitating
machine function outlining across modules. Additional codegen summaries
can be incorporated into separate sections as required. This patch
primarily establishes basic support for the reader and writer, similar
to llvm-profdata.

The high-level operations of llvm-cgdata are as follows:
1. It reads local raw codegen data from a custom section (for example,
__llvm_outline) embedded in native binary files
2. It merges local raw codegen data into an indexed codegen data,
complete with a suitable header.
3. It handles reading and writing of the indexed codegen data into a
standalone file.

This depends on #89792.
This is a patch for
https://discourse.llvm.org/t/rfc-enhanced-machine-outliner-part-2-thinlto-nolto/78753.

---------

Co-authored-by: Kyungwoo Lee <kyulee@fb.com>

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251410
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants