[PGO] Drive profile validator from opt #147418
Merged
Conversation
This file contains hidden or 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
This stack of pull requests is managed by Graphite. Learn more about stacking. |
mtrofin
force-pushed
the
users/mtrofin/07-07-drive_profile_validator_from_opt
branch
2 times, most recently
from
c0dbb3a to
05d7128
Compare
mtrofin
force-pushed
the
users/mtrofin/06-30-_pgo_profile_validation
branch
from
9c22f8c to
99e4436
Compare
mtrofin
force-pushed
the
users/mtrofin/07-07-drive_profile_validator_from_opt
branch
from
05d7128 to
003c225
Compare
mtrofin
force-pushed
the
users/mtrofin/06-30-_pgo_profile_validation
branch
from
99e4436 to
9200ff5
Compare
mtrofin
force-pushed
the
users/mtrofin/07-07-drive_profile_validator_from_opt
branch
from
003c225 to
92334f8
Compare
mtrofin
force-pushed
the
users/mtrofin/06-30-_pgo_profile_validation
branch
2 times, most recently
from
5d0b996 to
3df59d9
Compare
mtrofin
force-pushed
the
users/mtrofin/07-07-drive_profile_validator_from_opt
branch
2 times, most recently
from
12a21d8 to
a6e2214
Compare
mtrofin
force-pushed
the
users/mtrofin/06-30-_pgo_profile_validation
branch
2 times, most recently
from
7594643 to
f4d5327
Compare
mtrofin
force-pushed
the
users/mtrofin/07-07-drive_profile_validator_from_opt
branch
from
a6e2214 to
ed03e05
Compare
Base automatically changed from
users/mtrofin/06-30-_pgo_profile_validation
to
main
July 23, 2025 19:35
mtrofin
force-pushed
the
users/mtrofin/07-07-drive_profile_validator_from_opt
branch
2 times, most recently
from
de88f85 to
668bbc8
Compare
mtrofin
changed the title
mtrofin
requested review from
david-xl,
jdenny-ornl,
MatzeB,
snehasish and
teresajohnson
|
@llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesAdd option to Full diff: https://github.com/llvm/llvm-project/pull/147418.diff 4 Files Affected:
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify.ll b/llvm/test/Transforms/PGOProfile/prof-verify.ll
index c83475ab5d18e..3d984d88ffffb 100644
--- a/llvm/test/Transforms/PGOProfile/prof-verify.ll
+++ b/llvm/test/Transforms/PGOProfile/prof-verify.ll
@@ -3,6 +3,7 @@
; RUN: opt -passes=prof-inject %s -S -o - | FileCheck %s --check-prefix=INJECT
; RUN: not opt -passes=prof-verify %s -S -o - 2>&1 | FileCheck %s --check-prefix=VERIFY
; RUN: opt -passes=prof-inject,prof-verify %s --disable-output
+; RUN: opt -enable-profcheck %s -S -o - | FileCheck %s --check-prefix=INJECT
define void @foo(i32 %i) {
%c = icmp eq i32 %i, 0
diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp
index 7d168a6ceb17c..b9b8929a0f703 100644
--- a/llvm/tools/opt/NewPMDriver.cpp
+++ b/llvm/tools/opt/NewPMDriver.cpp
@@ -40,6 +40,7 @@
#include "llvm/Transforms/Instrumentation/AddressSanitizer.h"
#include "llvm/Transforms/Scalar/LoopPassManager.h"
#include "llvm/Transforms/Utils/Debugify.h"
+#include "llvm/Transforms/Utils/ProfileVerify.h"
using namespace llvm;
using namespace opt_tool;
@@ -356,7 +357,7 @@ bool llvm::runPassPipeline(
OutputKind OK, VerifierKind VK, bool ShouldPreserveAssemblyUseListOrder,
bool ShouldPreserveBitcodeUseListOrder, bool EmitSummaryIndex,
bool EmitModuleHash, bool EnableDebugify, bool VerifyDIPreserve,
- bool UnifiedLTO) {
+ bool EnableProfcheck, bool UnifiedLTO) {
auto FS = vfs::getRealFileSystem();
std::optional<PGOOptions> P;
switch (PGOKindFlag) {
@@ -487,7 +488,8 @@ bool llvm::runPassPipeline(
if (VerifyDIPreserve)
MPM.addPass(NewPMDebugifyPass(DebugifyMode::OriginalDebugInfo, "",
&DebugInfoBeforePass));
-
+ if (EnableProfcheck)
+ MPM.addPass(createModuleToFunctionPassAdaptor(ProfileInjectorPass()));
// Add passes according to the -passes options.
if (!PassPipeline.empty()) {
if (auto Err = PB.parsePassPipeline(MPM, PassPipeline)) {
@@ -504,6 +506,8 @@ bool llvm::runPassPipeline(
MPM.addPass(NewPMCheckDebugifyPass(
false, "", nullptr, DebugifyMode::OriginalDebugInfo,
&DebugInfoBeforePass, VerifyDIPreserveExport));
+ if (EnableProfcheck)
+ MPM.addPass(createModuleToFunctionPassAdaptor(ProfileVerifierPass()));
// Add any relevant output pass at the end of the pipeline.
switch (OK) {
diff --git a/llvm/tools/opt/NewPMDriver.h b/llvm/tools/opt/NewPMDriver.h
index 2daae571e72c2..6c21d6cae4e75 100644
--- a/llvm/tools/opt/NewPMDriver.h
+++ b/llvm/tools/opt/NewPMDriver.h
@@ -75,7 +75,7 @@ bool runPassPipeline(
bool ShouldPreserveAssemblyUseListOrder,
bool ShouldPreserveBitcodeUseListOrder, bool EmitSummaryIndex,
bool EmitModuleHash, bool EnableDebugify, bool VerifyDIPreserve,
- bool UnifiedLTO = false);
+ bool EnableProfcheck, bool UnifiedLTO = false);
} // namespace llvm
#endif
diff --git a/llvm/tools/opt/optdriver.cpp b/llvm/tools/opt/optdriver.cpp
index 892b63b5be251..7ff1f72a49c4f 100644
--- a/llvm/tools/opt/optdriver.cpp
+++ b/llvm/tools/opt/optdriver.cpp
@@ -217,6 +217,10 @@ static cl::opt<bool> VerifyDebugInfoPreserve(
cl::desc("Start the pipeline with collecting and end it with checking of "
"debug info preservation."));
+static cl::opt<bool> EnableProfileVerification(
+ "enable-profcheck", cl::init(false),
+ cl::desc("Start the pipeline with prof-inject and end it with prof-check"));
+
static cl::opt<std::string> ClDataLayout("data-layout",
cl::desc("data layout string to use"),
cl::value_desc("layout-string"),
@@ -746,7 +750,8 @@ extern "C" int optMain(
RemarksFile.get(), Pipeline, PluginList, PassBuilderCallbacks,
OK, VK, PreserveAssemblyUseListOrder,
PreserveBitcodeUseListOrder, EmitSummaryIndex, EmitModuleHash,
- EnableDebugify, VerifyDebugInfoPreserve, UnifiedLTO)
+ EnableDebugify, VerifyDebugInfoPreserve,
+ EnableProfileVerification, UnifiedLTO)
? 0
: 1;
}
|
|
@llvm/pr-subscribers-pgo Author: Mircea Trofin (mtrofin) ChangesAdd option to Full diff: https://github.com/llvm/llvm-project/pull/147418.diff 4 Files Affected:
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify.ll b/llvm/test/Transforms/PGOProfile/prof-verify.ll
index c83475ab5d18e..3d984d88ffffb 100644
--- a/llvm/test/Transforms/PGOProfile/prof-verify.ll
+++ b/llvm/test/Transforms/PGOProfile/prof-verify.ll
@@ -3,6 +3,7 @@
; RUN: opt -passes=prof-inject %s -S -o - | FileCheck %s --check-prefix=INJECT
; RUN: not opt -passes=prof-verify %s -S -o - 2>&1 | FileCheck %s --check-prefix=VERIFY
; RUN: opt -passes=prof-inject,prof-verify %s --disable-output
+; RUN: opt -enable-profcheck %s -S -o - | FileCheck %s --check-prefix=INJECT
define void @foo(i32 %i) {
%c = icmp eq i32 %i, 0
diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp
index 7d168a6ceb17c..b9b8929a0f703 100644
--- a/llvm/tools/opt/NewPMDriver.cpp
+++ b/llvm/tools/opt/NewPMDriver.cpp
@@ -40,6 +40,7 @@
#include "llvm/Transforms/Instrumentation/AddressSanitizer.h"
#include "llvm/Transforms/Scalar/LoopPassManager.h"
#include "llvm/Transforms/Utils/Debugify.h"
+#include "llvm/Transforms/Utils/ProfileVerify.h"
using namespace llvm;
using namespace opt_tool;
@@ -356,7 +357,7 @@ bool llvm::runPassPipeline(
OutputKind OK, VerifierKind VK, bool ShouldPreserveAssemblyUseListOrder,
bool ShouldPreserveBitcodeUseListOrder, bool EmitSummaryIndex,
bool EmitModuleHash, bool EnableDebugify, bool VerifyDIPreserve,
- bool UnifiedLTO) {
+ bool EnableProfcheck, bool UnifiedLTO) {
auto FS = vfs::getRealFileSystem();
std::optional<PGOOptions> P;
switch (PGOKindFlag) {
@@ -487,7 +488,8 @@ bool llvm::runPassPipeline(
if (VerifyDIPreserve)
MPM.addPass(NewPMDebugifyPass(DebugifyMode::OriginalDebugInfo, "",
&DebugInfoBeforePass));
-
+ if (EnableProfcheck)
+ MPM.addPass(createModuleToFunctionPassAdaptor(ProfileInjectorPass()));
// Add passes according to the -passes options.
if (!PassPipeline.empty()) {
if (auto Err = PB.parsePassPipeline(MPM, PassPipeline)) {
@@ -504,6 +506,8 @@ bool llvm::runPassPipeline(
MPM.addPass(NewPMCheckDebugifyPass(
false, "", nullptr, DebugifyMode::OriginalDebugInfo,
&DebugInfoBeforePass, VerifyDIPreserveExport));
+ if (EnableProfcheck)
+ MPM.addPass(createModuleToFunctionPassAdaptor(ProfileVerifierPass()));
// Add any relevant output pass at the end of the pipeline.
switch (OK) {
diff --git a/llvm/tools/opt/NewPMDriver.h b/llvm/tools/opt/NewPMDriver.h
index 2daae571e72c2..6c21d6cae4e75 100644
--- a/llvm/tools/opt/NewPMDriver.h
+++ b/llvm/tools/opt/NewPMDriver.h
@@ -75,7 +75,7 @@ bool runPassPipeline(
bool ShouldPreserveAssemblyUseListOrder,
bool ShouldPreserveBitcodeUseListOrder, bool EmitSummaryIndex,
bool EmitModuleHash, bool EnableDebugify, bool VerifyDIPreserve,
- bool UnifiedLTO = false);
+ bool EnableProfcheck, bool UnifiedLTO = false);
} // namespace llvm
#endif
diff --git a/llvm/tools/opt/optdriver.cpp b/llvm/tools/opt/optdriver.cpp
index 892b63b5be251..7ff1f72a49c4f 100644
--- a/llvm/tools/opt/optdriver.cpp
+++ b/llvm/tools/opt/optdriver.cpp
@@ -217,6 +217,10 @@ static cl::opt<bool> VerifyDebugInfoPreserve(
cl::desc("Start the pipeline with collecting and end it with checking of "
"debug info preservation."));
+static cl::opt<bool> EnableProfileVerification(
+ "enable-profcheck", cl::init(false),
+ cl::desc("Start the pipeline with prof-inject and end it with prof-check"));
+
static cl::opt<std::string> ClDataLayout("data-layout",
cl::desc("data layout string to use"),
cl::value_desc("layout-string"),
@@ -746,7 +750,8 @@ extern "C" int optMain(
RemarksFile.get(), Pipeline, PluginList, PassBuilderCallbacks,
OK, VK, PreserveAssemblyUseListOrder,
PreserveBitcodeUseListOrder, EmitSummaryIndex, EmitModuleHash,
- EnableDebugify, VerifyDebugInfoPreserve, UnifiedLTO)
+ EnableDebugify, VerifyDebugInfoPreserve,
+ EnableProfileVerification, UnifiedLTO)
? 0
: 1;
}
|
mtrofin
force-pushed
the
users/mtrofin/07-07-drive_profile_validator_from_opt
branch
from
668bbc8 to
0088835
Compare
snehasish
approved these changes
Jul 25, 2025
| @@ -101,6 +101,9 @@ | |||
| /* Define if LLVM is using tflite */ | |||
| #cmakedefine LLVM_HAVE_TFLITE | |||
|
|
|||
| /* Define if we want to check profile consistency in tests */ | |||
There was a problem hiding this comment.
Should we clarify that it's lit tests?
| @@ -1190,6 +1190,8 @@ if (LLVM_HAVE_TFLITE) | |||
| find_package(tensorflow-lite REQUIRED) | |||
| endif() | |||
|
|
|||
| set(LLVM_ENABLE_PROFCHECK OFF CACHE BOOL "Enable profile checking in test tools") | |||
There was a problem hiding this comment.
Maybe worth adding this option to llvm/docs/CMake.rst?
| @@ -55,6 +55,7 @@ config.linked_bye_extension = @LLVM_BYE_LINK_INTO_TOOLS@ | |||
| config.linked_exampleirtransforms_extension = @LLVM_EXAMPLEIRTRANSFORMS_LINK_INTO_TOOLS@ | |||
| config.have_tf_aot = @LLVM_HAVE_TF_AOT@ | |||
| config.have_tflite = @LLVM_HAVE_TFLITE@ | |||
| config.enable_profcheck = @LLVM_ENABLE_PROFCHECK@ | |||
There was a problem hiding this comment.
It doesn't look like you're using this anywhere?
There was a problem hiding this comment.
yes, but I have it there in case we need to apply to tests to disable them when this is enabled. It's fairly canonical, so figured I'd add it here.
Yes. Main trouble is that I'd have to pass the full path to opt, besides the extra arg. What I did here is (subjectively) simpler. |
mtrofin
force-pushed
the
users/mtrofin/07-07-drive_profile_validator_from_opt
branch
from
0088835 to
33f0cce
Compare
boomanaiden154
approved these changes
Jul 26, 2025
|
That articulates my hesitance basically. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/13318 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/5043 Here is the relevant piece of the build log for the reference |
|
Since all uses of this are in llvm/, isn't regular config.h enough? |
akuegel
pushed a commit
that referenced
this pull request
Jul 28, 2025
This PR adds the `#cmakedefine LLVM_ENABLE_PROFCHECK` in `llvm-config.h.cmake` introduced in #147418 to the copy of that file in the bazel overlay directory such that that define is also avalable in the bazel build. Not having the define broke the bazel build. Signed-off-by: Ingo Müller <[email protected]>
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Jul 28, 2025
Add option to `opt` to run the `ProfileInjectorPass` before the passes opt would run, and then `ProfileVerifierPass` after. This will then be a mode in which we run tests on a specialized buildbot, with the goal of finding passes that drop (and, later, corrupt) profile information.
|
optdriver uses it, and it already includes llvm-config. I'm offering this as a "I'm now confused what the rule is", actually - i.e. should config.h work here, or is it WAI? |
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.
Add option to
optto run theProfileInjectorPassbefore the passes opt would run, and thenProfileVerifierPassafter. This will then be a mode in which we run tests on a specialized buildbot, with the goal of finding passes that drop (and, later, corrupt) profile information.