Skip to content

Commit 0c6dc80

Browse files
authored
BalancedPartitioning: minor updates (#77568)
When LargestTraceSize is a power of two, createBPFunctionNodes does not allocate a group ID for Trace[LargestTraceSize-1] (as N is off by 1). Fix this and change floor+log2 to Log2_64. BalancedPartitioning::bisect can use unstable sort because `Nodes` contains distinct `InputOrderIndex`s. BalancedPartitioning::runIterations: use one DenseMap and simplify the node renumbering code.
1 parent bd5d41a commit 0c6dc80

File tree

3 files changed

+22
-19
lines changed

3 files changed

+22
-19
lines changed

llvm/lib/ProfileData/InstrProf.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -921,24 +921,24 @@ std::vector<BPFunctionNode> TemporalProfTraceTy::createBPFunctionNodes(
921921
if (Timestamp < Trace.FunctionNameRefs.size())
922922
FunctionIds.insert(Trace.FunctionNameRefs[Timestamp]);
923923

924-
int N = std::ceil(std::log2(LargestTraceSize));
924+
const int N = Log2_64(LargestTraceSize) + 1;
925925

926926
// TODO: We need to use the Trace.Weight field to give more weight to more
927927
// important utilities
928928
DenseMap<IDT, SmallVector<UtilityNodeT, 4>> FuncGroups;
929929
for (size_t TraceIdx = 0; TraceIdx < Traces.size(); TraceIdx++) {
930930
auto &Trace = Traces[TraceIdx].FunctionNameRefs;
931931
for (size_t Timestamp = 0; Timestamp < Trace.size(); Timestamp++) {
932-
for (int I = std::floor(std::log2(Timestamp + 1)); I < N; I++) {
933-
auto &FunctionId = Trace[Timestamp];
932+
for (int I = Log2_64(Timestamp + 1); I < N; I++) {
933+
auto FunctionId = Trace[Timestamp];
934934
UtilityNodeT GroupId = TraceIdx * N + I;
935935
FuncGroups[FunctionId].push_back(GroupId);
936936
}
937937
}
938938
}
939939

940940
std::vector<BPFunctionNode> Nodes;
941-
for (auto &Id : FunctionIds) {
941+
for (auto Id : FunctionIds) {
942942
auto &UNs = FuncGroups[Id];
943943
llvm::sort(UNs);
944944
UNs.erase(std::unique(UNs.begin(), UNs.end()), UNs.end());

llvm/lib/Support/BalancedPartitioning.cpp

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ void BalancedPartitioning::bisect(const FunctionNodeRange Nodes,
117117
if (NumNodes <= 1 || RecDepth >= Config.SplitDepth) {
118118
// We've reach the lowest level of the recursion tree. Fall back to the
119119
// original order and assign to buckets.
120-
llvm::stable_sort(Nodes, [](const auto &L, const auto &R) {
120+
llvm::sort(Nodes, [](const auto &L, const auto &R) {
121121
return L.InputOrderIndex < R.InputOrderIndex;
122122
});
123123
for (auto &N : Nodes)
@@ -167,26 +167,22 @@ void BalancedPartitioning::runIterations(const FunctionNodeRange Nodes,
167167
unsigned RightBucket,
168168
std::mt19937 &RNG) const {
169169
unsigned NumNodes = std::distance(Nodes.begin(), Nodes.end());
170-
DenseMap<BPFunctionNode::UtilityNodeT, unsigned> UtilityNodeDegree;
170+
DenseMap<BPFunctionNode::UtilityNodeT, unsigned> UtilityNodeIndex;
171171
for (auto &N : Nodes)
172172
for (auto &UN : N.UtilityNodes)
173-
++UtilityNodeDegree[UN];
173+
++UtilityNodeIndex[UN];
174174
// Remove utility nodes if they have just one edge or are connected to all
175175
// functions
176176
for (auto &N : Nodes)
177177
llvm::erase_if(N.UtilityNodes, [&](auto &UN) {
178-
return UtilityNodeDegree[UN] <= 1 || UtilityNodeDegree[UN] >= NumNodes;
178+
return UtilityNodeIndex[UN] == 1 || UtilityNodeIndex[UN] == NumNodes;
179179
});
180180

181181
// Renumber utility nodes so they can be used to index into Signatures
182-
DenseMap<BPFunctionNode::UtilityNodeT, unsigned> UtilityNodeIndex;
183-
for (auto &N : Nodes)
184-
for (auto &UN : N.UtilityNodes)
185-
if (!UtilityNodeIndex.count(UN))
186-
UtilityNodeIndex[UN] = UtilityNodeIndex.size();
182+
UtilityNodeIndex.clear();
187183
for (auto &N : Nodes)
188184
for (auto &UN : N.UtilityNodes)
189-
UN = UtilityNodeIndex[UN];
185+
UN = UtilityNodeIndex.insert({UN, UtilityNodeIndex.size()}).first->second;
190186

191187
// Initialize signatures
192188
SignaturesT Signatures(/*Size=*/UtilityNodeIndex.size());

llvm/unittests/ProfileData/BPFunctionNodeTest.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,25 @@ void PrintTo(const BPFunctionNode &Node, std::ostream *OS) {
2424
}
2525

2626
TEST(BPFunctionNodeTest, Basic) {
27-
auto Nodes = TemporalProfTraceTy::createBPFunctionNodes({
28-
TemporalProfTraceTy({0, 1, 2, 3, 4}),
29-
TemporalProfTraceTy({4, 2}),
30-
});
31-
3227
auto NodeIs = [](BPFunctionNode::IDT Id,
3328
ArrayRef<BPFunctionNode::UtilityNodeT> UNs) {
3429
return AllOf(Field("Id", &BPFunctionNode::Id, Id),
3530
Field("UtilityNodes", &BPFunctionNode::UtilityNodes,
3631
UnorderedElementsAreArray(UNs)));
3732
};
3833

34+
auto Nodes = TemporalProfTraceTy::createBPFunctionNodes({
35+
TemporalProfTraceTy({0, 1, 2, 3}),
36+
});
37+
EXPECT_THAT(Nodes,
38+
UnorderedElementsAre(NodeIs(0, {0, 1, 2}), NodeIs(1, {1, 2}),
39+
NodeIs(2, {1, 2}), NodeIs(3, {2})));
40+
41+
Nodes = TemporalProfTraceTy::createBPFunctionNodes({
42+
TemporalProfTraceTy({0, 1, 2, 3, 4}),
43+
TemporalProfTraceTy({4, 2}),
44+
});
45+
3946
EXPECT_THAT(Nodes,
4047
UnorderedElementsAre(NodeIs(0, {0, 1, 2}), NodeIs(1, {1, 2}),
4148
NodeIs(2, {1, 2, 4, 5}), NodeIs(3, {2}),

0 commit comments

Comments
 (0)