Skip to content

Commit a18ee8b

Browse files
committed
[MC] Make .pseudo_probe created sections deterministic after D91878
MCPseudoProbeSections::emit iterates over MCProbeDivisions and creates sections. When the map key is MCSymbol *, the iteration order is not stable. The underlying BumpPtrAllocator largely decreases the flakiness. That said, two elements may sit in two different allocations from BumpPtrAllocator, with an unpredictable order. Under tcmalloc, llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll fails about 7 times per 1000 runs.
1 parent 4914d33 commit a18ee8b

File tree

3 files changed

+16
-11
lines changed

3 files changed

+16
-11
lines changed

llvm/include/llvm/MC/MCPseudoProbe.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,13 @@
5454
#ifndef LLVM_MC_MCPSEUDOPROBE_H
5555
#define LLVM_MC_MCPSEUDOPROBE_H
5656

57+
#include "llvm/ADT/DenseMap.h"
5758
#include "llvm/ADT/DenseSet.h"
5859
#include "llvm/ADT/SmallVector.h"
5960
#include "llvm/ADT/StringRef.h"
6061
#include "llvm/IR/PseudoProbe.h"
6162
#include "llvm/Support/ErrorOr.h"
6263
#include <list>
63-
#include <map>
6464
#include <memory>
6565
#include <string>
6666
#include <tuple>
@@ -299,8 +299,9 @@ class MCPseudoProbeSections {
299299
MCProbeDivisions[FuncSym].addPseudoProbe(Probe, InlineStack);
300300
}
301301

302-
// TODO: Sort by getOrdinal to ensure a determinstic section order
303-
using MCProbeDivisionMap = std::map<MCSymbol *, MCPseudoProbeInlineTree>;
302+
// The addresses of MCPseudoProbeInlineTree are used by the tree structure and
303+
// need to be stable.
304+
using MCProbeDivisionMap = std::unordered_map<MCSymbol *, MCPseudoProbeInlineTree>;
304305

305306
private:
306307
// A collection of MCPseudoProbe for each function. The MCPseudoProbes are

llvm/lib/MC/MCPseudoProbe.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,13 @@ void MCPseudoProbeInlineTree::emit(MCObjectStreamer *MCOS,
209209

210210
void MCPseudoProbeSections::emit(MCObjectStreamer *MCOS) {
211211
MCContext &Ctx = MCOS->getContext();
212-
for (auto &ProbeSec : MCProbeDivisions) {
213-
const auto *FuncSym = ProbeSec.first;
214-
const auto &Root = ProbeSec.second;
212+
SmallVector<std::pair<MCSymbol *, MCPseudoProbeInlineTree *>> Vec;
213+
Vec.reserve(MCProbeDivisions.size());
214+
for (auto &ProbeSec : MCProbeDivisions)
215+
Vec.emplace_back(ProbeSec.first, &ProbeSec.second);
216+
llvm::sort(Vec, [](auto A, auto B) { return A.second->Guid < B.second->Guid; });
217+
for (auto [FuncSym, RootPtr] : Vec) {
218+
const auto &Root = *RootPtr;
215219
if (auto *S = Ctx.getObjectFileInfo()->getPseudoProbeSection(
216220
FuncSym->getSection())) {
217221
// Switch to the .pseudoprobe section or a comdat group.

llvm/test/Transforms/SampleProfile/pseudo-probe-emit.ll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,15 +109,15 @@ entry:
109109
; CHECK-SEC: [ 5] .text.foo2 PROGBITS {{.*}} 00 AX 0 0 16
110110
; CHECK-SEC: [ 8] .text.foo3 PROGBITS {{.*}} 00 AXG 0 0 16
111111
; CHECK-SEC-COUNT-3: .pseudo_probe_desc PROGBITS
112-
; CHECK-SEC: .pseudo_probe PROGBITS {{.*}} 00 L 3 0 1
112+
; CHECK-SEC: .pseudo_probe PROGBITS {{.*}} 00 LG 8 0 1
113113
; CHECK-SEC-NEXT: .pseudo_probe PROGBITS {{.*}} 00 L 5 0 1
114-
; CHECK-SEC-NEXT: .pseudo_probe PROGBITS {{.*}} 00 LG 8 0 1
114+
; CHECK-SEC-NEXT: .pseudo_probe PROGBITS {{.*}} 00 L 3 0 1
115115
; CHECK-SEC-NOT: .rela.pseudo_probe
116116

117117
; CHECK-SEC: COMDAT group section [ 7] `.group' [foo3] contains 2 sections:
118118
; CHECK-SEC-NEXT: [Index] Name
119119
; CHECK-SEC-NEXT: [ 8] .text.foo3
120-
; CHECK-SEC-NEXT: [ 21] .pseudo_probe
120+
; CHECK-SEC-NEXT: [ 19] .pseudo_probe
121121
; CHECK-SEC-EMPTY:
122122
; CHECK-SEC-NEXT: COMDAT group section [ 10] `.group' [.pseudo_probe_desc_foo] contains 1 sections:
123123
; CHECK-SEC-NEXT: [Index] Name
@@ -137,9 +137,9 @@ entry:
137137
; CHECK-SEC2: [ 5] .text PROGBITS {{.*}} 00 AX 0 0 16
138138
; CHECK-SEC2: [ 8] .text PROGBITS {{.*}} 00 AXG 0 0 16
139139
; CHECK-SEC2-COUNT-3: .pseudo_probe_desc PROGBITS
140-
; CHECK-SEC2: .pseudo_probe PROGBITS {{.*}} 00 L 3 0 1
140+
; CHECK-SEC2: .pseudo_probe PROGBITS {{.*}} 00 LG 8 0 1
141141
; CHECK-SEC2-NEXT: .pseudo_probe PROGBITS {{.*}} 00 L 5 0 1
142-
; CHECK-SEC2-NEXT: .pseudo_probe PROGBITS {{.*}} 00 LG 8 0 1
142+
; CHECK-SEC2-NEXT: .pseudo_probe PROGBITS {{.*}} 00 L 3 0 1
143143
; CHECK-SEC2-NOT: .rela.pseudo_probe
144144

145145
!llvm.dbg.cu = !{!0}

0 commit comments

Comments
 (0)