[sanitizer][NFCI] Add Options parameter to LowerAllowCheckPass #122765
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 is glue code to convert LowerAllowCheckPass from a FUNCTION_PASS to FUNCTION_PASS_WITH_PARAMS. The parameters are currently unused. Future work will plumb `-fsanitize-skip-hot-cutoff` (introduced in llvm#121619) to LowerAllowCheckOptions.
thurstond
force-pushed
the
lowerallowcheck_options
branch
from
a8d8a33 to
2938d91
Compare
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Thurston Dang (thurstond) ChangesThis is glue code to convert LowerAllowCheckPass from a FUNCTION_PASS to FUNCTION_PASS_WITH_PARAMS. The parameters are currently unused. Future work will plumb Full diff: https://github.com/llvm/llvm-project/pull/122765.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 79e6bf3d24dffb..bcdd849787dccb 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -795,11 +795,12 @@ static void addSanitizers(const Triple &TargetTriple,
}
if (LowerAllowCheckPass::IsRequested()) {
+ LowerAllowCheckOptions Opts;
// We want to call it after inline, which is about OptimizerEarlyEPCallback.
- PB.registerOptimizerEarlyEPCallback([](ModulePassManager &MPM,
- OptimizationLevel Level,
- ThinOrFullLTOPhase Phase) {
- MPM.addPass(createModuleToFunctionPassAdaptor(LowerAllowCheckPass()));
+ PB.registerOptimizerEarlyEPCallback([&Opts](ModulePassManager &MPM,
+ OptimizationLevel Level,
+ ThinOrFullLTOPhase Phase) {
+ MPM.addPass(createModuleToFunctionPassAdaptor(LowerAllowCheckPass(Opts)));
});
}
}
diff --git a/llvm/include/llvm/Transforms/Instrumentation/LowerAllowCheckPass.h b/llvm/include/llvm/Transforms/Instrumentation/LowerAllowCheckPass.h
index af974818fec5f3..6fe9f2eccbc3af 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/LowerAllowCheckPass.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/LowerAllowCheckPass.h
@@ -20,13 +20,22 @@
namespace llvm {
+struct LowerAllowCheckOptions {
+ std::vector<double> placeholder; // TODO: cutoffs
+};
+
// This pass is responsible for removing optional traps, like llvm.ubsantrap
// from the hot code.
class LowerAllowCheckPass : public PassInfoMixin<LowerAllowCheckPass> {
public:
+ explicit LowerAllowCheckPass(LowerAllowCheckOptions Options)
+ : Options(Options) {};
PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
static bool IsRequested();
+
+private:
+ LowerAllowCheckOptions Options;
};
} // namespace llvm
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index f923d5aabe0a0e..aeebfb75bcb5d5 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -821,6 +821,21 @@ Expected<EmbedBitcodeOptions> parseEmbedBitcodePassOptions(StringRef Params) {
return Result;
}
+Expected<LowerAllowCheckOptions>
+parseLowerAllowCheckPassOptions(StringRef Params) {
+ LowerAllowCheckOptions Result;
+ while (!Params.empty()) {
+ StringRef ParamName;
+ std::tie(ParamName, Params) = Params.split(';');
+
+ return make_error<StringError>(
+ formatv("invalid LowerAllowCheck pass parameter '{0}' ", ParamName)
+ .str(),
+ inconvertibleErrorCode());
+ }
+ return Result;
+}
+
Expected<MemorySanitizerOptions> parseMSanPassOptions(StringRef Params) {
MemorySanitizerOptions Result;
while (!Params.empty()) {
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 1021d7fcd92474..7cbea023f2129f 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -402,7 +402,6 @@ FUNCTION_PASS("loop-load-elim", LoopLoadEliminationPass())
FUNCTION_PASS("loop-simplify", LoopSimplifyPass())
FUNCTION_PASS("loop-sink", LoopSinkPass())
FUNCTION_PASS("loop-versioning", LoopVersioningPass())
-FUNCTION_PASS("lower-allow-check", LowerAllowCheckPass())
FUNCTION_PASS("lower-atomic", LowerAtomicPass())
FUNCTION_PASS("lower-constant-intrinsics", LowerConstantIntrinsicsPass())
FUNCTION_PASS("lower-expect", LowerExpectIntrinsicPass())
@@ -553,6 +552,10 @@ FUNCTION_PASS_WITH_PARAMS(
parseLoopVectorizeOptions,
"no-interleave-forced-only;interleave-forced-only;no-vectorize-forced-only;"
"vectorize-forced-only")
+FUNCTION_PASS_WITH_PARAMS(
+ "lower-allow-check", "LowerAllowCheckPass",
+ [](LowerAllowCheckOptions Opts) { return LowerAllowCheckPass(Opts); },
+ parseLowerAllowCheckPassOptions, "")
FUNCTION_PASS_WITH_PARAMS(
"lower-matrix-intrinsics", "LowerMatrixIntrinsicsPass",
[](bool Minimal) { return LowerMatrixIntrinsicsPass(Minimal); },
|
Member
|
@llvm/pr-subscribers-llvm-transforms Author: Thurston Dang (thurstond) ChangesThis is glue code to convert LowerAllowCheckPass from a FUNCTION_PASS to FUNCTION_PASS_WITH_PARAMS. The parameters are currently unused. Future work will plumb Full diff: https://github.com/llvm/llvm-project/pull/122765.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 79e6bf3d24dffb..bcdd849787dccb 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -795,11 +795,12 @@ static void addSanitizers(const Triple &TargetTriple,
}
if (LowerAllowCheckPass::IsRequested()) {
+ LowerAllowCheckOptions Opts;
// We want to call it after inline, which is about OptimizerEarlyEPCallback.
- PB.registerOptimizerEarlyEPCallback([](ModulePassManager &MPM,
- OptimizationLevel Level,
- ThinOrFullLTOPhase Phase) {
- MPM.addPass(createModuleToFunctionPassAdaptor(LowerAllowCheckPass()));
+ PB.registerOptimizerEarlyEPCallback([&Opts](ModulePassManager &MPM,
+ OptimizationLevel Level,
+ ThinOrFullLTOPhase Phase) {
+ MPM.addPass(createModuleToFunctionPassAdaptor(LowerAllowCheckPass(Opts)));
});
}
}
diff --git a/llvm/include/llvm/Transforms/Instrumentation/LowerAllowCheckPass.h b/llvm/include/llvm/Transforms/Instrumentation/LowerAllowCheckPass.h
index af974818fec5f3..6fe9f2eccbc3af 100644
--- a/llvm/include/llvm/Transforms/Instrumentation/LowerAllowCheckPass.h
+++ b/llvm/include/llvm/Transforms/Instrumentation/LowerAllowCheckPass.h
@@ -20,13 +20,22 @@
namespace llvm {
+struct LowerAllowCheckOptions {
+ std::vector<double> placeholder; // TODO: cutoffs
+};
+
// This pass is responsible for removing optional traps, like llvm.ubsantrap
// from the hot code.
class LowerAllowCheckPass : public PassInfoMixin<LowerAllowCheckPass> {
public:
+ explicit LowerAllowCheckPass(LowerAllowCheckOptions Options)
+ : Options(Options) {};
PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
static bool IsRequested();
+
+private:
+ LowerAllowCheckOptions Options;
};
} // namespace llvm
diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index f923d5aabe0a0e..aeebfb75bcb5d5 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -821,6 +821,21 @@ Expected<EmbedBitcodeOptions> parseEmbedBitcodePassOptions(StringRef Params) {
return Result;
}
+Expected<LowerAllowCheckOptions>
+parseLowerAllowCheckPassOptions(StringRef Params) {
+ LowerAllowCheckOptions Result;
+ while (!Params.empty()) {
+ StringRef ParamName;
+ std::tie(ParamName, Params) = Params.split(';');
+
+ return make_error<StringError>(
+ formatv("invalid LowerAllowCheck pass parameter '{0}' ", ParamName)
+ .str(),
+ inconvertibleErrorCode());
+ }
+ return Result;
+}
+
Expected<MemorySanitizerOptions> parseMSanPassOptions(StringRef Params) {
MemorySanitizerOptions Result;
while (!Params.empty()) {
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 1021d7fcd92474..7cbea023f2129f 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -402,7 +402,6 @@ FUNCTION_PASS("loop-load-elim", LoopLoadEliminationPass())
FUNCTION_PASS("loop-simplify", LoopSimplifyPass())
FUNCTION_PASS("loop-sink", LoopSinkPass())
FUNCTION_PASS("loop-versioning", LoopVersioningPass())
-FUNCTION_PASS("lower-allow-check", LowerAllowCheckPass())
FUNCTION_PASS("lower-atomic", LowerAtomicPass())
FUNCTION_PASS("lower-constant-intrinsics", LowerConstantIntrinsicsPass())
FUNCTION_PASS("lower-expect", LowerExpectIntrinsicPass())
@@ -553,6 +552,10 @@ FUNCTION_PASS_WITH_PARAMS(
parseLoopVectorizeOptions,
"no-interleave-forced-only;interleave-forced-only;no-vectorize-forced-only;"
"vectorize-forced-only")
+FUNCTION_PASS_WITH_PARAMS(
+ "lower-allow-check", "LowerAllowCheckPass",
+ [](LowerAllowCheckOptions Opts) { return LowerAllowCheckPass(Opts); },
+ parseLowerAllowCheckPassOptions, "")
FUNCTION_PASS_WITH_PARAMS(
"lower-matrix-intrinsics", "LowerMatrixIntrinsicsPass",
[](bool Minimal) { return LowerMatrixIntrinsicsPass(Minimal); },
|
vitalybuka
reviewed
Jan 13, 2025
llvm/include/llvm/Transforms/Instrumentation/LowerAllowCheckPass.h
Outdated
Show resolved
Hide resolved
vitalybuka
reviewed
Jan 13, 2025
llvm/include/llvm/Transforms/Instrumentation/LowerAllowCheckPass.h
Outdated
Show resolved
Hide resolved
vitalybuka
approved these changes
Jan 13, 2025
…d int> for cutoffs
vitalybuka
reviewed
Jan 13, 2025
| } | ||
|
|
||
| Expected<LowerAllowCheckPass::Options> | ||
| parseLowerAllowCheckPassOptions(StringRef Params) { |
Collaborator
There was a problem hiding this comment.
if we have parsing, we need
void printPipeline(raw_ostream &OS,
function_ref<StringRef(StringRef)> MapClassName2PassName);
in the pass
Contributor
Author
There was a problem hiding this comment.
Thanks, noted. I will add it in a follow-up patch that performs parsing (and unparsing).
vitalybuka
approved these changes
Jan 13, 2025
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/137/builds/11521 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/10393 Here is the relevant piece of the build log for the reference |
thurstond
added a commit
that referenced
this pull request
Jan 14, 2025
…s" (#122833) Reverts #122765 Reason: buildbot breakage (https://lab.llvm.org/buildbot/#/builders/46/builds/10393) ``` z:\b\llvm-clang-x86_64-sie-win\build\bin\clang.exe -cc1 -internal-isystem Z:\b\llvm-clang-x86_64-sie-win\build\lib\clang\20\include -nostdsysteminc -triple x86_64-pc-linux-gnu -emit-llvm -o - Z:\b\llvm-clang-x86_64-sie-win\llvm-project\clang\test\CodeGen\allow-ubsan-check-inline.c -fsanitize=signed-integer-overflow -mllvm -ubsan-guard-checks -O3 -mllvm -lower-allow-check-random-rate=1 -Rpass=lower-allow-check -Rpass-missed=lower-allow-check -fno-inline 2>&1 | z:\b\llvm-clang-x86_64-sie-win\build\bin\filecheck.exe Z:\b\llvm-clang-x86_64-sie-win\llvm-project\clang\test\CodeGen\allow-ubsan-check-inline.c --check-prefixes=NOINL --implicit-check-not="remark:" # executed command: 'z:\b\llvm-clang-x86_64-sie-win\build\bin\clang.exe' -cc1 -internal-isystem 'Z:\b\llvm-clang-x86_64-sie-win\build\lib\clang\20\include' -nostdsysteminc -triple x86_64-pc-linux-gnu -emit-llvm -o - 'Z:\b\llvm-clang-x86_64-sie-win\llvm-project\clang\test\CodeGen\allow-ubsan-check-inline.c' -fsanitize=signed-integer-overflow -mllvm -ubsan-guard-checks -O3 -mllvm -lower-allow-check-random-rate=1 -Rpass=lower-allow-check -Rpass-missed=lower-allow-check -fno-inline # note: command had no output on stdout or stderr # error: command failed with exit status: 0xc0000409 # executed command: 'z:\b\llvm-clang-x86_64-sie-win\build\bin\filecheck.exe' 'Z:\b\llvm-clang-x86_64-sie-win\llvm-project\clang\test\CodeGen\allow-ubsan-check-inline.c' --check-prefixes=NOINL --implicit-check-not=remark: # .---command stderr------------ # | FileCheck error: '<stdin>' is empty. # | FileCheck command line: z:\b\llvm-clang-x86_64-sie-win\build\bin\filecheck.exe Z:\b\llvm-clang-x86_64-sie-win\llvm-project\clang\test\CodeGen\allow-ubsan-check-inline.c --check-prefixes=NOINL --implicit-check-not=remark: # `----------------------------- # error: command failed with exit status: 2 ```
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 14, 2025
…lowCheckPass" (#122833) Reverts llvm/llvm-project#122765 Reason: buildbot breakage (https://lab.llvm.org/buildbot/#/builders/46/builds/10393) ``` z:\b\llvm-clang-x86_64-sie-win\build\bin\clang.exe -cc1 -internal-isystem Z:\b\llvm-clang-x86_64-sie-win\build\lib\clang\20\include -nostdsysteminc -triple x86_64-pc-linux-gnu -emit-llvm -o - Z:\b\llvm-clang-x86_64-sie-win\llvm-project\clang\test\CodeGen\allow-ubsan-check-inline.c -fsanitize=signed-integer-overflow -mllvm -ubsan-guard-checks -O3 -mllvm -lower-allow-check-random-rate=1 -Rpass=lower-allow-check -Rpass-missed=lower-allow-check -fno-inline 2>&1 | z:\b\llvm-clang-x86_64-sie-win\build\bin\filecheck.exe Z:\b\llvm-clang-x86_64-sie-win\llvm-project\clang\test\CodeGen\allow-ubsan-check-inline.c --check-prefixes=NOINL --implicit-check-not="remark:" # executed command: 'z:\b\llvm-clang-x86_64-sie-win\build\bin\clang.exe' -cc1 -internal-isystem 'Z:\b\llvm-clang-x86_64-sie-win\build\lib\clang\20\include' -nostdsysteminc -triple x86_64-pc-linux-gnu -emit-llvm -o - 'Z:\b\llvm-clang-x86_64-sie-win\llvm-project\clang\test\CodeGen\allow-ubsan-check-inline.c' -fsanitize=signed-integer-overflow -mllvm -ubsan-guard-checks -O3 -mllvm -lower-allow-check-random-rate=1 -Rpass=lower-allow-check -Rpass-missed=lower-allow-check -fno-inline # note: command had no output on stdout or stderr # error: command failed with exit status: 0xc0000409 # executed command: 'z:\b\llvm-clang-x86_64-sie-win\build\bin\filecheck.exe' 'Z:\b\llvm-clang-x86_64-sie-win\llvm-project\clang\test\CodeGen\allow-ubsan-check-inline.c' --check-prefixes=NOINL --implicit-check-not=remark: # .---command stderr------------ # | FileCheck error: '<stdin>' is empty. # | FileCheck command line: z:\b\llvm-clang-x86_64-sie-win\build\bin\filecheck.exe Z:\b\llvm-clang-x86_64-sie-win\llvm-project\clang\test\CodeGen\allow-ubsan-check-inline.c --check-prefixes=NOINL --implicit-check-not=remark: # `----------------------------- # error: command failed with exit status: 2 ```
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/11927 Here is the relevant piece of the build log for the reference |
thurstond
added a commit
to thurstond/llvm-project
that referenced
this pull request
Jan 15, 2025
…ss" (llvm#122833) This reverts commit 1515caf (llvm#122833) i.e., relands 7d8b4eb (llvm#122765), with the addition of std::move to fix a stack use-after-scope error.
thurstond
added a commit
to thurstond/llvm-project
that referenced
this pull request
Jan 22, 2025
…ss" (llvm#122833) This reverts commit 1515caf (llvm#122833) i.e., relands 7d8b4eb (llvm#122765), with the addition of std::move to fix a stack use-after-scope error.
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 22, 2025
…llowCheckPass" (#122833) (#122994) This reverts commit 1515caf (llvm/llvm-project#122833) i.e., relands 7d8b4eb (llvm/llvm-project#122765), with LowerAllowCheckPass::Options moved inside the callback to fix a stack use-after-scope error. --------- Co-authored-by: Vitaly Buka <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This is glue code to convert LowerAllowCheckPass from a FUNCTION_PASS to FUNCTION_PASS_WITH_PARAMS. The parameters are currently unused.
Future work will plumb
-fsanitize-skip-hot-cutoff(introduced in #121619) to LowerAllowCheckOptions.