Address feedback #2 from ellishg + clean-up header inclusion

llvm · Jul 6, 2024 · c6fa20b · c6fa20b
1 parent 40eaed6
commit c6fa20b
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 21 deletions.
diff --git a/llvm/include/llvm/CodeGenData/OutlinedHashTree.h b/llvm/include/llvm/CodeGenData/OutlinedHashTree.h
@@ -15,6 +15,7 @@
 #ifndef LLVM_CODEGENDATA_OUTLINEDHASHTREE_H
 #define LLVM_CODEGENDATA_OUTLINEDHASHTREE_H
 
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StableHashing.h"
 #include "llvm/ObjectYAML/YAML.h"
 #include "llvm/Support/raw_ostream.h"

diff --git a/llvm/include/llvm/CodeGenData/OutlinedHashTreeRecord.h b/llvm/include/llvm/CodeGenData/OutlinedHashTreeRecord.h
@@ -16,7 +16,6 @@
 #ifndef LLVM_CODEGENDATA_OUTLINEDHASHTREERECORD_H
 #define LLVM_CODEGENDATA_OUTLINEDHASHTREERECORD_H
 
-#include "llvm/ADT/DenseMap.h"
 #include "llvm/CodeGenData/OutlinedHashTree.h"
 
 namespace llvm {

diff --git a/llvm/lib/CodeGenData/OutlinedHashTree.cpp b/llvm/lib/CodeGenData/OutlinedHashTree.cpp
@@ -14,9 +14,6 @@
 
 #include "llvm/CodeGenData/OutlinedHashTree.h"
 
-#include <stack>
-#include <tuple>
-
 #define DEBUG_TYPE "outlined-hash-tree"
 
 using namespace llvm;
@@ -38,9 +35,10 @@ void OutlinedHashTree::walkGraph(NodeCallbackFn CallbackNode,
       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();
+      SmallVector<std::pair<stable_hash, const HashNode *>> SortedSuccessors;
+      for (const auto &[Hash, Successor] : Current->Successors)
+        SortedSuccessors.emplace_back(Hash, Successor.get());
+      llvm::sort(SortedSuccessors);
       for (const auto &P : SortedSuccessors)
         HandleNext(P.second);
     } else {
@@ -60,7 +58,7 @@ size_t OutlinedHashTree::size(bool GetTerminalCountOnly) const {
 
 size_t OutlinedHashTree::depth() const {
   size_t Size = 0;
-  std::unordered_map<const HashNode *, size_t> DepthMap;
+  DenseMap<const HashNode *, size_t> DepthMap;
   walkGraph([&Size, &DepthMap](
                 const HashNode *N) { Size = std::max(Size, DepthMap[N]); },
             [&DepthMap](const HashNode *Src, const HashNode *Dst) {

diff --git a/llvm/lib/CodeGenData/OutlinedHashTreeRecord.cpp b/llvm/lib/CodeGenData/OutlinedHashTreeRecord.cpp
@@ -14,7 +14,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/CodeGenData/OutlinedHashTreeRecord.h"
-#include "llvm/CodeGenData/OutlinedHashTree.h"
 #include "llvm/ObjectYAML/YAML.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/EndianStream.h"

diff --git a/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp b/llvm/unittests/CodeGenData/OutlinedHashTreeTest.cpp
@@ -16,30 +16,30 @@ namespace {
 
 TEST(OutlinedHashTreeTest, Empty) {
   OutlinedHashTree HashTree;
-  ASSERT_TRUE(HashTree.empty());
+  EXPECT_TRUE(HashTree.empty());
   // The header node is always present.
-  ASSERT_TRUE(HashTree.size() == 1);
-  ASSERT_TRUE(HashTree.depth() == 0);
+  EXPECT_EQ(HashTree.size(), 1);
+  EXPECT_EQ(HashTree.depth(), 0);
 }
 
 TEST(OutlinedHashTreeTest, Insert) {
   OutlinedHashTree HashTree;
   HashTree.insert({{1, 2, 3}, 1});
   // The node count is 4 (including the root node).
-  ASSERT_TRUE(HashTree.size() == 4);
+  EXPECT_EQ(HashTree.size(), 4);
   // The terminal count is 1.
-  ASSERT_TRUE(HashTree.size(/*GetTerminalCountOnly=*/true) == 1);
+  EXPECT_EQ(HashTree.size(/*GetTerminalCountOnly=*/true), 1);
   // The depth is 3.
-  ASSERT_TRUE(HashTree.depth() == 3);
+  EXPECT_EQ(HashTree.depth(), 3);
 
   HashTree.clear();
-  ASSERT_TRUE(HashTree.empty());
+  EXPECT_TRUE(HashTree.empty());
 
   HashTree.insert({{1, 2, 3}, 1});
   HashTree.insert({{1, 2, 4}, 2});
   // The nodes of 1 and 2 are shared with the same prefix.
   // The nodes are root, 1, 2, 3 and 4, whose counts are 5.
-  ASSERT_TRUE(HashTree.size() == 5);
+  EXPECT_EQ(HashTree.size(), 5);
 }
 
 TEST(OutlinedHashTreeTest, Find) {
@@ -48,11 +48,11 @@ TEST(OutlinedHashTreeTest, Find) {
   HashTree.insert({{1, 2, 3}, 2});
 
   // The node count does not change as the same sequences are added.
-  ASSERT_TRUE(HashTree.size() == 4);
+  EXPECT_EQ(HashTree.size(), 4);
   // The terminal counts are accumulated from two same sequences.
-  ASSERT_TRUE(HashTree.find({1, 2, 3}));
-  ASSERT_TRUE(HashTree.find({1, 2, 3}).value() == 3);
-  ASSERT_FALSE(HashTree.find({1, 2}));
+  EXPECT_TRUE(HashTree.find({1, 2, 3}));
+  EXPECT_EQ(HashTree.find({1, 2, 3}).value(), 3);
+  EXPECT_FALSE(HashTree.find({1, 2}));
 }
 
 TEST(OutlinedHashTreeTest, Merge) {