[ctx_prof] "Use" support for pre-thinlink. #101338
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There is currently no plan to support contextual profiling use in a non- ThinLTO scenario. In the pre-link phase, we only instrument and then immediately bail out to let the linker group functions under an entrypoint in the same module as the entrypoint. We don't actually care what the profile contains - just that we want to use a contextual profile. After that, in post-thinlink, we require the profile be passed again so we can actually use it. The earlier instrumentation will be used to match counter values. While the feature is in development, we add a hidden flag for the use scenario, but we can eventually tie it to the `PGOOptions` mechanism. We will use the same flag in both pre- and post-thinlink, because it simplifies things - usually the post-thinlink args are the same as the ones for pre-. This, despite the flag being basically treated as a boolean in pre-thinlink.
|
@llvm/pr-subscribers-pgo Author: Mircea Trofin (mtrofin) ChangesThere is currently no plan to support contextual profiling use in a non- ThinLTO scenario. In the pre-link phase, we only instrument and then immediately bail out to let the linker group functions under an entrypoint in the same module as the entrypoint. We don't actually care what the profile contains - just that we want to use a contextual profile. After that, in post-thinlink, we require the profile be passed again so we can actually use it. The earlier instrumentation will be used to match counter values. While the feature is in development, we add a hidden flag for the use scenario, but we can eventually tie it to the Full diff: https://github.com/llvm/llvm-project/pull/101338.diff 2 Files Affected:
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index a6118726945e8..e5fe202e21b1d 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -304,6 +304,10 @@ static cl::opt<bool> UseLoopVersioningLICM(
"enable-loop-versioning-licm", cl::init(false), cl::Hidden,
cl::desc("Enable the experimental Loop Versioning LICM pass"));
+static cl::opt<std::string>
+ UseCtxProfile("use-ctx-profile", cl::init(""), cl::Hidden,
+ cl::desc("Use the specified contextual profile file"));
+
namespace llvm {
extern cl::opt<bool> EnableMemProfContextDisambiguation;
@@ -1176,8 +1180,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
// Enable contextual profiling instrumentation.
const bool IsCtxProfGen = !IsPGOInstrGen && IsPreLink &&
PGOCtxProfLoweringPass::isContextualIRPGOEnabled();
+ const bool IsCtxProfUse = !UseCtxProfile.empty() && !PGOOpt &&
+ Phase == ThinOrFullLTOPhase::ThinLTOPreLink;
- if (IsPGOInstrGen || IsPGOInstrUse || IsMemprofUse || IsCtxProfGen)
+ if (IsPGOInstrGen || IsPGOInstrUse || IsMemprofUse || IsCtxProfGen ||
+ IsCtxProfUse)
addPreInlinerPasses(MPM, Level, Phase);
// Add all the requested passes for instrumentation PGO, if requested.
@@ -1187,8 +1194,12 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
/*IsCS=*/false, PGOOpt->AtomicCounterUpdate,
PGOOpt->ProfileFile, PGOOpt->ProfileRemappingFile,
PGOOpt->FS);
- } else if (IsCtxProfGen) {
+ } else if (IsCtxProfGen || IsCtxProfUse) {
MPM.addPass(PGOInstrumentationGen(false));
+ // In pre-link, we just want the instrumented IR. We use the contextual
+ // profile in the post-thinlink phase.
+ if (IsCtxProfUse)
+ return MPM;
addPostPGOLoopRotation(MPM, Level);
MPM.addPass(PGOCtxProfLoweringPass());
}
@@ -1655,6 +1666,11 @@ PassBuilder::buildThinLTOPreLinkDefaultPipeline(OptimizationLevel Level) {
// can.
MPM.addPass(buildModuleSimplificationPipeline(
Level, ThinOrFullLTOPhase::ThinLTOPreLink));
+ // In pre-link, for ctx prof use, we stop here with an instrumented IR. We let
+ // thinlto use the contextual info to perform imports; then use the contextual
+ // profile in the post-thinlink phase.
+ if (!UseCtxProfile.empty() && !PGOOpt)
+ return MPM;
// Run partial inlining pass to partially inline functions that have
// large bodies.
diff --git a/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll b/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll
new file mode 100644
index 0000000000000..b50a815be5abf
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll
@@ -0,0 +1,38 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; There is no profile, but that's OK because the prelink does not care about
+; the content of the profile, just that we intend to use one.
+; There is no scenario currently of doing ctx profile use without thinlto.
+;
+; RUN: opt -passes='thinlto-pre-link<O2>' -use-ctx-profile=something_that_does_not_exist %s -S | FileCheck %s
+
+declare void @bar()
+
+define void @foo(i32 %a, ptr %fct) {
+; CHECK-LABEL: define void @foo(
+; CHECK-SAME: i32 [[A:%.*]], ptr [[FCT:%.*]]) local_unnamed_addr {
+; CHECK-NEXT: [[T:%.*]] = icmp eq i32 [[A]], 0
+; CHECK-NEXT: br i1 [[T]], label %[[YES:.*]], label %[[NO:.*]]
+; CHECK: [[YES]]:
+; CHECK-NEXT: call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 1)
+; CHECK-NEXT: [[TMP1:%.*]] = ptrtoint ptr [[FCT]] to i64
+; CHECK-NEXT: call void @llvm.instrprof.value.profile(ptr @__profn_foo, i64 728453322856651412, i64 [[TMP1]], i32 0, i32 0)
+; CHECK-NEXT: call void [[FCT]](i32 0)
+; CHECK-NEXT: br label %[[EXIT:.*]]
+; CHECK: [[NO]]:
+; CHECK-NEXT: call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 0)
+; CHECK-NEXT: call void @bar()
+; CHECK-NEXT: br label %[[EXIT]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: ret void
+;
+ %t = icmp eq i32 %a, 0
+ br i1 %t, label %yes, label %no
+yes:
+ call void %fct(i32 %a)
+ br label %exit
+no:
+ call void @bar()
+ br label %exit
+exit:
+ ret void
+}
|
|
@llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesThere is currently no plan to support contextual profiling use in a non- ThinLTO scenario. In the pre-link phase, we only instrument and then immediately bail out to let the linker group functions under an entrypoint in the same module as the entrypoint. We don't actually care what the profile contains - just that we want to use a contextual profile. After that, in post-thinlink, we require the profile be passed again so we can actually use it. The earlier instrumentation will be used to match counter values. While the feature is in development, we add a hidden flag for the use scenario, but we can eventually tie it to the Full diff: https://github.com/llvm/llvm-project/pull/101338.diff 2 Files Affected:
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index a6118726945e8..e5fe202e21b1d 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -304,6 +304,10 @@ static cl::opt<bool> UseLoopVersioningLICM(
"enable-loop-versioning-licm", cl::init(false), cl::Hidden,
cl::desc("Enable the experimental Loop Versioning LICM pass"));
+static cl::opt<std::string>
+ UseCtxProfile("use-ctx-profile", cl::init(""), cl::Hidden,
+ cl::desc("Use the specified contextual profile file"));
+
namespace llvm {
extern cl::opt<bool> EnableMemProfContextDisambiguation;
@@ -1176,8 +1180,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
// Enable contextual profiling instrumentation.
const bool IsCtxProfGen = !IsPGOInstrGen && IsPreLink &&
PGOCtxProfLoweringPass::isContextualIRPGOEnabled();
+ const bool IsCtxProfUse = !UseCtxProfile.empty() && !PGOOpt &&
+ Phase == ThinOrFullLTOPhase::ThinLTOPreLink;
- if (IsPGOInstrGen || IsPGOInstrUse || IsMemprofUse || IsCtxProfGen)
+ if (IsPGOInstrGen || IsPGOInstrUse || IsMemprofUse || IsCtxProfGen ||
+ IsCtxProfUse)
addPreInlinerPasses(MPM, Level, Phase);
// Add all the requested passes for instrumentation PGO, if requested.
@@ -1187,8 +1194,12 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
/*IsCS=*/false, PGOOpt->AtomicCounterUpdate,
PGOOpt->ProfileFile, PGOOpt->ProfileRemappingFile,
PGOOpt->FS);
- } else if (IsCtxProfGen) {
+ } else if (IsCtxProfGen || IsCtxProfUse) {
MPM.addPass(PGOInstrumentationGen(false));
+ // In pre-link, we just want the instrumented IR. We use the contextual
+ // profile in the post-thinlink phase.
+ if (IsCtxProfUse)
+ return MPM;
addPostPGOLoopRotation(MPM, Level);
MPM.addPass(PGOCtxProfLoweringPass());
}
@@ -1655,6 +1666,11 @@ PassBuilder::buildThinLTOPreLinkDefaultPipeline(OptimizationLevel Level) {
// can.
MPM.addPass(buildModuleSimplificationPipeline(
Level, ThinOrFullLTOPhase::ThinLTOPreLink));
+ // In pre-link, for ctx prof use, we stop here with an instrumented IR. We let
+ // thinlto use the contextual info to perform imports; then use the contextual
+ // profile in the post-thinlink phase.
+ if (!UseCtxProfile.empty() && !PGOOpt)
+ return MPM;
// Run partial inlining pass to partially inline functions that have
// large bodies.
diff --git a/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll b/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll
new file mode 100644
index 0000000000000..b50a815be5abf
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll
@@ -0,0 +1,38 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; There is no profile, but that's OK because the prelink does not care about
+; the content of the profile, just that we intend to use one.
+; There is no scenario currently of doing ctx profile use without thinlto.
+;
+; RUN: opt -passes='thinlto-pre-link<O2>' -use-ctx-profile=something_that_does_not_exist %s -S | FileCheck %s
+
+declare void @bar()
+
+define void @foo(i32 %a, ptr %fct) {
+; CHECK-LABEL: define void @foo(
+; CHECK-SAME: i32 [[A:%.*]], ptr [[FCT:%.*]]) local_unnamed_addr {
+; CHECK-NEXT: [[T:%.*]] = icmp eq i32 [[A]], 0
+; CHECK-NEXT: br i1 [[T]], label %[[YES:.*]], label %[[NO:.*]]
+; CHECK: [[YES]]:
+; CHECK-NEXT: call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 1)
+; CHECK-NEXT: [[TMP1:%.*]] = ptrtoint ptr [[FCT]] to i64
+; CHECK-NEXT: call void @llvm.instrprof.value.profile(ptr @__profn_foo, i64 728453322856651412, i64 [[TMP1]], i32 0, i32 0)
+; CHECK-NEXT: call void [[FCT]](i32 0)
+; CHECK-NEXT: br label %[[EXIT:.*]]
+; CHECK: [[NO]]:
+; CHECK-NEXT: call void @llvm.instrprof.increment(ptr @__profn_foo, i64 728453322856651412, i32 2, i32 0)
+; CHECK-NEXT: call void @bar()
+; CHECK-NEXT: br label %[[EXIT]]
+; CHECK: [[EXIT]]:
+; CHECK-NEXT: ret void
+;
+ %t = icmp eq i32 %a, 0
+ br i1 %t, label %yes, label %no
+yes:
+ call void %fct(i32 %a)
+ br label %exit
+no:
+ call void @bar()
+ br label %exit
+exit:
+ ret void
+}
|
| @@ -1176,8 +1180,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, | |||
| // Enable contextual profiling instrumentation. | |||
| const bool IsCtxProfGen = !IsPGOInstrGen && IsPreLink && | |||
| PGOCtxProfLoweringPass::isContextualIRPGOEnabled(); | |||
| const bool IsCtxProfUse = !UseCtxProfile.empty() && !PGOOpt && | |||
| Phase == ThinOrFullLTOPhase::ThinLTOPreLink; | |||
There was a problem hiding this comment.
Just use the IsPreLink flag like you did above for IsCtxProfGen? What happens for regular LTO, which is the other case where IsPreLink could be true? If not supported there then consider adding an assert.
There was a problem hiding this comment.
But IsPreLink is actually "Is not post-link". None also passes. With the current way, we also exclude the full LTO case, IIUC.
We could rename IsPreLink because it's really IsNotPostThinLink, wdyt?
There was a problem hiding this comment.
Yes I think it means essentially not post link. But I'm wondering why you check IsPreLink for IsCtxProfGen above and the more restrictive check here.
There was a problem hiding this comment.
The instrumentation phase can happen without thinlto.
| MPM.addPass(PGOInstrumentationGen(false)); | ||
| // In pre-link, we just want the instrumented IR. We use the contextual |
There was a problem hiding this comment.
Is the instrumentation removed in the post-LTO link after using it for matching? Maybe add that to the comment to clarify
| @@ -1176,8 +1180,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, | |||
| // Enable contextual profiling instrumentation. | |||
| const bool IsCtxProfGen = !IsPGOInstrGen && IsPreLink && | |||
| PGOCtxProfLoweringPass::isContextualIRPGOEnabled(); | |||
| const bool IsCtxProfUse = !UseCtxProfile.empty() && !PGOOpt && | |||
There was a problem hiding this comment.
Rather than checking!PGOOpt here and further below, should it be an error to combine these options?
There was a problem hiding this comment.
maybe, but I'm not seeing the usual error handling mechanism here; also, eventually I want to use PGOOpt (after some refactoring, because right now it's state is quite involved, see its ctor). While ctx_prof is in development, seems overkill to solve all these.
mtrofin
commented
Aug 2, 2024
banach-space
pushed a commit
to banach-space/llvm-project
that referenced
this pull request
Aug 7, 2024
There is currently no plan to support contextual profiling use in a non- ThinLTO scenario. In the pre-link phase, we only instrument and then immediately bail out to let the linker group functions under an entrypoint in the same module as the entrypoint. We don't actually care what the profile contains - just that we want to use a contextual profile. After that, in post-thinlink, we require the profile be passed again so we can actually use it. The earlier instrumentation will be used to match counter values. While the feature is in development, we add a hidden flag for the use scenario, but we can eventually tie it to the `PGOOptions` mechanism. We will use the same flag in both pre- and post-thinlink, because it simplifies things - usually the post-thinlink args are the same as the ones for pre-. This, despite the flag being basically treated as a boolean in pre-thinlink.
mtrofin
added a commit
to mtrofin/llvm-project
that referenced
this pull request
Aug 8, 2024
Didn't notice in llvm#101338 that the instrumentation in `llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll` was actually incorrect.
mtrofin
added a commit
that referenced
this pull request
Aug 8, 2024
Didn't notice in #101338 that the instrumentation in `llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll` was actually incorrect.
bwendling
pushed a commit
to bwendling/llvm-project
that referenced
this pull request
Aug 15, 2024
Didn't notice in llvm#101338 that the instrumentation in `llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll` was actually incorrect.
kstoimenov
pushed a commit
to kstoimenov/llvm-project
that referenced
this pull request
Aug 15, 2024
There is currently no plan to support contextual profiling use in a non- ThinLTO scenario. In the pre-link phase, we only instrument and then immediately bail out to let the linker group functions under an entrypoint in the same module as the entrypoint. We don't actually care what the profile contains - just that we want to use a contextual profile. After that, in post-thinlink, we require the profile be passed again so we can actually use it. The earlier instrumentation will be used to match counter values. While the feature is in development, we add a hidden flag for the use scenario, but we can eventually tie it to the `PGOOptions` mechanism. We will use the same flag in both pre- and post-thinlink, because it simplifies things - usually the post-thinlink args are the same as the ones for pre-. This, despite the flag being basically treated as a boolean in pre-thinlink.
kstoimenov
pushed a commit
to kstoimenov/llvm-project
that referenced
this pull request
Aug 15, 2024
Didn't notice in llvm#101338 that the instrumentation in `llvm/test/Transforms/PGOProfile/ctx-prof-use-prelink.ll` was actually incorrect.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is currently no plan to support contextual profiling use in a non- ThinLTO scenario.
In the pre-link phase, we only instrument and then immediately bail out to let the linker group functions under an entrypoint in the same module as the entrypoint. We don't actually care what the profile contains - just that we want to use a contextual profile.
After that, in post-thinlink, we require the profile be passed again so we can actually use it. The earlier instrumentation will be used to match counter values.
While the feature is in development, we add a hidden flag for the use scenario, but we can eventually tie it to the
PGOOptionsmechanism. We will use the same flag in both pre- and post-thinlink, because it simplifies things - usually the post-thinlink args are the same as the ones for pre-. This, despite the flag being basically treated as a boolean in pre-thinlink.Issue #89287