Skip to content

Commit c61e22f

Browse files
committed
[ctxprof] Only prune the profile in modules containing only context trees
1 parent 5aae0ee commit c61e22f

File tree

6 files changed

+130
-22
lines changed

6 files changed

+130
-22
lines changed

llvm/include/llvm/Analysis/CtxProfAnalysis.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ class PGOContextualProfile {
3333
FunctionInfo(StringRef Name) : Name(Name) {}
3434
};
3535
PGOCtxProfile Profiles;
36+
37+
// True if this module is a post-thinlto module containing just functions
38+
// participating in one or more contextual profiles.
39+
bool IsInSpecializedModule = false;
40+
3641
// For the GUIDs in this module, associate metadata about each function which
3742
// we'll need when we maintain the profiles during IPO transformations.
3843
std::map<GlobalValue::GUID, FunctionInfo> FuncInfo;
@@ -56,6 +61,8 @@ class PGOContextualProfile {
5661

5762
const PGOCtxProfile &profiles() const { return Profiles; }
5863

64+
bool isInSpecializedModule() const { return IsInSpecializedModule; }
65+
5966
bool isFunctionKnown(const Function &F) const {
6067
return getDefinedFunctionGUID(F) != 0;
6168
}

llvm/lib/Analysis/CtxProfAnalysis.cpp

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "llvm/Analysis/CtxProfAnalysis.h"
15+
#include "llvm/ADT/APInt.h"
1516
#include "llvm/ADT/STLExtras.h"
1617
#include "llvm/IR/Analysis.h"
1718
#include "llvm/IR/IntrinsicInst.h"
@@ -20,6 +21,7 @@
2021
#include "llvm/ProfileData/PGOCtxProfReader.h"
2122
#include "llvm/Support/CommandLine.h"
2223
#include "llvm/Support/MemoryBuffer.h"
24+
#include "llvm/Support/Path.h"
2325

2426
#define DEBUG_TYPE "ctx_prof"
2527

@@ -95,26 +97,42 @@ PGOContextualProfile CtxProfAnalysis::run(Module &M,
9597
return {};
9698
}
9799

98-
DenseSet<GlobalValue::GUID> ProfileRootsInModule;
99-
for (const auto &F : M)
100-
if (!F.isDeclaration())
101-
if (auto GUID = AssignGUIDPass::getGUID(F);
102-
MaybeProfiles->Contexts.find(GUID) != MaybeProfiles->Contexts.end())
103-
ProfileRootsInModule.insert(GUID);
104-
105-
// Trim first the roots that aren't in this module.
106-
for (auto &[RootGuid, _] :
107-
llvm::make_early_inc_range(MaybeProfiles->Contexts))
108-
if (!ProfileRootsInModule.contains(RootGuid))
109-
MaybeProfiles->Contexts.erase(RootGuid);
110-
// If none of the roots are in the module, we have no profile (for this
111-
// module)
112-
if (MaybeProfiles->Contexts.empty())
113-
return {};
114-
115-
// OK, so we have a valid profile and it's applicable to roots in this module.
100+
// FIXME: We should drive this from ThinLTO, but for the time being, use the
101+
// module name as indicator.
102+
// We want to *only* keep the contextual profiles in modules that capture
103+
// context trees. That allows us to compute specific PSIs, for example.
104+
auto DetermineRootsInModule = [&M]() -> const DenseSet<GlobalValue::GUID> {
105+
DenseSet<GlobalValue::GUID> ProfileRootsInModule;
106+
auto ModName = M.getName();
107+
auto Filename = sys::path::filename(ModName);
108+
// Drop the file extension.
109+
Filename = Filename.substr(0, Filename.find_last_of('.'));
110+
// See if it parses
111+
APInt Guid;
112+
// getAsInteger returns true if there are more chars to read other than the
113+
// integer. So the "false" test is what we want.
114+
if (!Filename.getAsInteger(0, Guid))
115+
ProfileRootsInModule.insert(Guid.getZExtValue());
116+
return ProfileRootsInModule;
117+
};
118+
const auto ProfileRootsInModule = DetermineRootsInModule();
116119
PGOContextualProfile Result;
117120

121+
// the logic from here on allows for modules that contain - by design - more
122+
// than one root. We currently don't support that, because the determination
123+
// happens based on the module name matching the root guid, but the logic can
124+
// avoid assuming that.
125+
if (!ProfileRootsInModule.empty()) {
126+
Result.IsInSpecializedModule = true;
127+
// Trim first the roots that aren't in this module.
128+
for (auto &[RootGuid, _] :
129+
llvm::make_early_inc_range(MaybeProfiles->Contexts))
130+
if (!ProfileRootsInModule.contains(RootGuid))
131+
MaybeProfiles->Contexts.erase(RootGuid);
132+
// we can also drop the flat profiles
133+
MaybeProfiles->FlatProfiles.clear();
134+
}
135+
118136
for (const auto &F : M) {
119137
if (F.isDeclaration())
120138
continue;

llvm/lib/Transforms/IPO/ElimAvailExtern.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ EliminateAvailableExternallyPass::run(Module &M, ModuleAnalysisManager &MAM) {
134134
// for this contextual information. Eliding it in favor of the original would
135135
// undo these optimizations.
136136
if (!eliminateAvailableExternally(
137-
M, /*Convert=*/(CtxProf && !CtxProf->contexts().empty())))
137+
M, /*Convert=*/(CtxProf && CtxProf->isInSpecializedModule())))
138138
return PreservedAnalyses::all();
139139
return PreservedAnalyses::none();
140140
}

llvm/test/Analysis/CtxProfAnalysis/load.ll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ Function Info:
4646
Current Profile:
4747

4848
Contexts:
49+
- Guid: 12341
50+
TotalRootEntryCount: 90
51+
Counters: [ 9 ]
4952
- Guid: 11872291593386833696
5053
TotalRootEntryCount: 4
5154
Counters: [ 1 ]
@@ -57,6 +60,7 @@ Contexts:
5760
Counters: [ 5 ]
5861

5962
Flat Profile:
63+
12341 : 9
6064
728453322856651412 : 6 7
6165
11872291593386833696 : 1
6266
12074870348631550642 : 5
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
; REQUIRES: x86_64-linux
2+
;
3+
; Check that we don't prune the contextual profile, unless the module name
4+
; matches the guid of the root.
5+
;
6+
; RUN: rm -rf %t
7+
; RUN: split-file %s %t
8+
; RUN: llvm-ctxprof-util fromYAML --input=%t/profile.yaml --output=%t/profile.ctxprofdata
9+
;
10+
; RUN: cp %t/example.ll %t/1234.ll
11+
; RUN: cp %t/example.ll %t/0x4d2.ll
12+
;
13+
; RUN: opt -passes='require<ctx-prof-analysis>,print<ctx-prof-analysis>' \
14+
; RUN: -use-ctx-profile=%t/profile.ctxprofdata \
15+
; RUN: -ctx-profile-printer-level=everything \
16+
; RUN: %t/example.ll -S 2>&1 | FileCheck %s
17+
18+
; RUN: opt -passes='require<ctx-prof-analysis>,print<ctx-prof-analysis>' \
19+
; RUN: -use-ctx-profile=%t/profile.ctxprofdata \
20+
; RUN: -ctx-profile-printer-level=everything \
21+
; RUN: %t/not-matching.ll -S 2>&1 | FileCheck %s
22+
23+
; RUN: opt -passes='require<ctx-prof-analysis>,print<ctx-prof-analysis>' \
24+
; RUN: -use-ctx-profile=%t/profile.ctxprofdata \
25+
; RUN: -ctx-profile-printer-level=everything \
26+
; RUN: %t/0x4d2.ll -S 2>&1 | FileCheck %s --check-prefix=PRUNED
27+
28+
; CHECK: Contexts:
29+
; CHECK: - Guid: 1234
30+
; CHECK: - Guid: 5678
31+
; CHECK: FlatProfiles:
32+
; PRUNED-NOT: - Guid: 5678
33+
; PRUNED-NOT: FlatProfiles
34+
;
35+
; pick a large GUID that would be negative, if signed, to test a few ways the
36+
; file name may be formatted.
37+
;--- profile.yaml
38+
Contexts:
39+
- Guid: 1234
40+
TotalRootEntryCount: 24
41+
Counters: [9]
42+
Callsites: -
43+
- Guid: 1000
44+
Counters: [6, 7]
45+
46+
- Guid: 5678
47+
TotalRootEntryCount: 24
48+
Counters: [9]
49+
Callsites: -
50+
- Guid: 1000
51+
Counters: [6, 7]
52+
FlatProfiles:
53+
- Guid: 777
54+
Counters: [2]
55+
;--- example.ll
56+
define void @an_entrypoint(i32 %a) !guid !0 {
57+
ret void
58+
}
59+
60+
attributes #0 = { noinline }
61+
!0 = !{ i64 1234 }
62+
63+
;--- not-matching.ll
64+
define void @an_entrypoint(i32 %a) !guid !0 {
65+
ret void
66+
}
67+
68+
attributes #0 = { noinline }
69+
!0 = !{ i64 1000 }

llvm/test/Transforms/EliminateAvailableExternally/transform-to-local.ll

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,26 @@
11
; REQUIRES: asserts
2-
; RUN: opt -passes=elim-avail-extern -avail-extern-to-local -stats -S 2>&1 < %s | FileCheck %s
2+
; RUN: rm -rf %t
3+
; RUN: mkdir %t
4+
; RUN: cp %s %t/1234.ll
5+
;
6+
; default behavior
7+
; RUN: opt -passes=elim-avail-extern -stats -S 2>&1 %s | FileCheck %s --check-prefix=NOOP
8+
;
9+
; check the -avail-extern-to-local flag works as intended
10+
; RUN: opt -passes=elim-avail-extern -avail-extern-to-local -stats -S 2>&1 %t/1234.ll | FileCheck %s
11+
; RUN: opt -passes=elim-avail-extern -avail-extern-to-local -stats -S 2>&1 %s | FileCheck %s
312
;
413
; RUN: echo '{"Contexts": [{"Guid":1234, "TotalRootEntryCount": 5, "Counters": [1]}]}' | llvm-ctxprof-util fromYAML --input=- --output=%t_profile.ctxprofdata
514
;
615
; Because we pass a contextual profile with a root defined in this module, we expect the outcome to be the same as-if
716
; we passed -avail-extern-to-local, i.e. available_externally don't get elided and instead get converted to local linkage
8-
; RUN: opt -passes='assign-guid,require<ctx-prof-analysis>,elim-avail-extern' -use-ctx-profile=%t_profile.ctxprofdata -stats -S 2>&1 < %s | FileCheck %s
17+
;
18+
; RUN: opt -passes='assign-guid,require<ctx-prof-analysis>,elim-avail-extern' -use-ctx-profile=%t_profile.ctxprofdata -stats -S 2>&1 %t/1234.ll | FileCheck %s
919

1020
; If the profile doesn't apply to this module, available_externally won't get converted to internal linkage, and will be
1121
; removed instead.
1222
; RUN: echo '{"Contexts": [{"Guid":5678, "TotalRootEntryCount": 3, "Counters": [1]}]}' | llvm-ctxprof-util fromYAML --input=- --output=%t_profile_bad.ctxprofdata
13-
; RUN: opt -passes='assign-guid,require<ctx-prof-analysis>,elim-avail-extern' -use-ctx-profile=%t_profile_bad.ctxprofdata -stats -S 2>&1 < %s | FileCheck %s --check-prefix=NOOP
23+
; RUN: opt -passes='assign-guid,require<ctx-prof-analysis>,elim-avail-extern' -use-ctx-profile=%t_profile_bad.ctxprofdata -stats -S %s 2>&1 | FileCheck %s --check-prefix=NOOP
1424

1525
declare void @call_out(ptr %fct)
1626

0 commit comments

Comments
 (0)