From c6fa20ba7adcc5f83e6684f3c59863f826ea6248 Mon Sep 17 00:00:00 2001 From: Kyungwoo Lee Date: Tue, 18 Jun 2024 17:03:50 -0700 Subject: [PATCH] Address feedback #2 from ellishg + clean-up header inclusion --- .../llvm/CodeGenData/OutlinedHashTree.h | 1 + .../llvm/CodeGenData/OutlinedHashTreeRecord.h | 1 - llvm/lib/CodeGenData/OutlinedHashTree.cpp | 12 ++++------ .../CodeGenData/OutlinedHashTreeRecord.cpp | 1 - .../CodeGenData/OutlinedHashTreeTest.cpp | 24 +++++++++---------- 5 files changed, 18 insertions(+), 21 deletions(-) diff --git a/llvm/include/llvm/CodeGenData/OutlinedHashTree.h b/llvm/include/llvm/CodeGenData/OutlinedHashTree.h index c40038cd8c517..2c8a9288f8a8c 100644 --- 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 index 2960e31960448..55938edfe0b3d 100644 --- 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 index cb985aa87afcf..d64098098de62 100644 --- a/llvm/lib/CodeGenData/OutlinedHashTree.cpp +++ b/llvm/lib/CodeGenData/OutlinedHashTree.cpp @@ -14,9 +14,6 @@ #include "llvm/CodeGenData/OutlinedHashTree.h" -#include -#include - #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 SortedSuccessors; - for (const auto &P : Current->Successors) - SortedSuccessors[P.first] = P.second.get(); + SmallVector> 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 DepthMap; + DenseMap 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 index da4db7e9e69f1..996a57fd5e713 100644 --- 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 index 5fdfa60673b7f..4ab5566cf817b 100644 --- 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) {