-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[profcheck] Don't verify generated global ctors/dtors #170597
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[profcheck] Don't verify generated global ctors/dtors #170597
Conversation
|
@llvm/pr-subscribers-pgo Author: Mircea Trofin (mtrofin) ChangesFunctions listed in Full diff: https://github.com/llvm/llvm-project/pull/170597.diff 8 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/ProfileVerify.h b/llvm/include/llvm/Transforms/Utils/ProfileVerify.h
index 5c9c44c23bc01..0ba611b574ed1 100644
--- a/llvm/include/llvm/Transforms/Utils/ProfileVerify.h
+++ b/llvm/include/llvm/Transforms/Utils/ProfileVerify.h
@@ -13,6 +13,7 @@
#ifndef LLVM_TRANSFORMS_UTILS_PROFILEVERIFY_H
#define LLVM_TRANSFORMS_UTILS_PROFILEVERIFY_H
+#include "llvm/ADT/DenseSet.h"
#include "llvm/IR/Analysis.h"
#include "llvm/IR/PassManager.h"
#include "llvm/Support/Compiler.h"
@@ -29,8 +30,11 @@ class ProfileInjectorPass : public PassInfoMixin<ProfileInjectorPass> {
/// in conjunction with the ProfileInjectorPass. MD_prof "unknown" is considered
/// valid (i.e. !{!"unknown"})
class ProfileVerifierPass : public PassInfoMixin<ProfileVerifierPass> {
+ DenseSet<const Function *> IgnoreList;
+ PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
+
public:
- LLVM_ABI PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
+ LLVM_ABI PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM);
};
} // namespace llvm
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index e9874ecd553ee..cf998f29ef38c 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -149,6 +149,7 @@ MODULE_PASS("print<ir2vec>", IR2VecPrinterPass(errs()))
MODULE_PASS("print<ir2vec-vocab>", IR2VecVocabPrinterPass(errs()))
MODULE_PASS("print<module-debuginfo>", ModuleDebugInfoPrinterPass(errs()))
MODULE_PASS("print<reg-usage>", PhysicalRegisterUsageInfoPrinterPass(errs()))
+MODULE_PASS("prof-verify", ProfileVerifierPass())
MODULE_PASS("pseudo-probe", SampleProfileProbePass(TM))
MODULE_PASS("pseudo-probe-update", PseudoProbeUpdatePass())
MODULE_PASS("recompute-globalsaa", RecomputeGlobalsAAPass())
@@ -526,7 +527,6 @@ FUNCTION_PASS("print<scev-division>", SCEVDivisionPrinterPass(errs()))
FUNCTION_PASS("print<stack-safety-local>", StackSafetyPrinterPass(errs()))
FUNCTION_PASS("print<uniformity>", UniformityInfoPrinterPass(errs()))
FUNCTION_PASS("prof-inject", ProfileInjectorPass())
-FUNCTION_PASS("prof-verify", ProfileVerifierPass())
FUNCTION_PASS("reassociate", ReassociatePass())
FUNCTION_PASS("redundant-dbg-inst-elim", RedundantDbgInstEliminationPass())
FUNCTION_PASS("replace-with-veclib", ReplaceWithVeclib())
diff --git a/llvm/lib/Transforms/Utils/ProfileVerify.cpp b/llvm/lib/Transforms/Utils/ProfileVerify.cpp
index ca6bd91b1f530..d9fb879bac74c 100644
--- a/llvm/lib/Transforms/Utils/ProfileVerify.cpp
+++ b/llvm/lib/Transforms/Utils/ProfileVerify.cpp
@@ -11,13 +11,19 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/Analysis/BranchProbabilityInfo.h"
#include "llvm/IR/Analysis.h"
+#include "llvm/IR/Constants.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Function.h"
+#include "llvm/IR/GlobalValue.h"
+#include "llvm/IR/GlobalVariable.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/PassManager.h"
#include "llvm/IR/ProfDataUtils.h"
#include "llvm/Support/BranchProbability.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/CommandLine.h"
using namespace llvm;
@@ -189,11 +195,39 @@ PreservedAnalyses ProfileInjectorPass::run(Function &F,
return PreservedAnalyses::none();
}
+PreservedAnalyses ProfileVerifierPass::run(Module &M,
+ ModuleAnalysisManager &MAM) {
+ auto PopulateIgnoreList = [&](StringRef GVName) {
+ if (const auto *CT = M.getGlobalVariable(GVName))
+ if (const auto *CA =
+ dyn_cast_if_present<ConstantArray>(CT->getInitializer()))
+ for (const auto &Elt : CA->operands())
+ if (const auto *CS = dyn_cast<ConstantStruct>(Elt))
+ if (CS->getNumOperands() >= 2 && CS->getOperand(1))
+ if (const auto *F = dyn_cast<Function>(
+ CS->getOperand(1)->stripPointerCasts()))
+ IgnoreList.insert(F);
+ };
+ PopulateIgnoreList("llvm.global_ctors");
+ PopulateIgnoreList("llvm.global_dtors");
+ struct Wrapper : PassInfoMixin<Wrapper> {
+ ProfileVerifierPass &PVP;
+ PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM) {
+ return PVP.run(F, FAM);
+ }
+ explicit Wrapper(ProfileVerifierPass &PVP) : PVP(PVP) {}
+ };
+
+ return createModuleToFunctionPassAdaptor(Wrapper(*this)).run(M, MAM);
+}
+
PreservedAnalyses ProfileVerifierPass::run(Function &F,
FunctionAnalysisManager &FAM) {
// skip purely asm functions
if (isAsmOnly(F))
return PreservedAnalyses::all();
+ if (IgnoreList.contains(&F))
+ return PreservedAnalyses::all();
const auto EntryCount = F.getEntryCount(/*AllowSynthetic=*/true);
if (!EntryCount) {
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify-known-cold.ll b/llvm/test/Transforms/PGOProfile/prof-verify-known-cold.ll
index 7875300006761..9fa2732fbc6b5 100644
--- a/llvm/test/Transforms/PGOProfile/prof-verify-known-cold.ll
+++ b/llvm/test/Transforms/PGOProfile/prof-verify-known-cold.ll
@@ -1,6 +1,6 @@
; Test prof-verify for functions explicitly marked as cold
-; RUN: opt -passes=prof-inject,prof-verify %s -o - 2>&1 | FileCheck %s
+; RUN: opt -passes=function(prof-inject),module(prof-verify) %s -o - 2>&1 | FileCheck %s
define void @foo(i32 %i) !prof !0 {
%c = icmp eq i32 %i, 0
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify.ll b/llvm/test/Transforms/PGOProfile/prof-verify.ll
index 75d1e6a3db571..90a20b68ab23c 100644
--- a/llvm/test/Transforms/PGOProfile/prof-verify.ll
+++ b/llvm/test/Transforms/PGOProfile/prof-verify.ll
@@ -6,7 +6,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 -passes=function(prof-inject),module(prof-verify) %s --disable-output
; RUN: opt -enable-profcheck %s -S -o - | FileCheck %s --check-prefix=INJECT
define void @foo(i32 %i) !prof !0 {
diff --git a/llvm/test/Transforms/PGOProfile/profcheck-llvm.global_ctors.ll b/llvm/test/Transforms/PGOProfile/profcheck-llvm.global_ctors.ll
new file mode 100644
index 0000000000000..84c680fc158ac
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/profcheck-llvm.global_ctors.ll
@@ -0,0 +1,13 @@
+; RUN: opt -passes=prof-verify %s -o - 2>&1 | FileCheck %s
+@llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 0, ptr @ctor, ptr null }]
+@llvm.global_dtors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 0, ptr @dtor, ptr null }]
+
+define internal void @ctor() {
+ ret void
+}
+
+define internal void @dtor() {
+ ret void
+}
+
+; CHECK-NOT: Profile verification failed: function entry count missing
diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp
index 3209b652b44b4..eaa1d8f958a4d 100644
--- a/llvm/tools/opt/NewPMDriver.cpp
+++ b/llvm/tools/opt/NewPMDriver.cpp
@@ -520,7 +520,7 @@ bool llvm::runPassPipeline(
false, "", nullptr, DebugifyMode::OriginalDebugInfo,
&DebugInfoBeforePass, VerifyDIPreserveExport));
if (EnableProfcheck)
- MPM.addPass(createModuleToFunctionPassAdaptor(ProfileVerifierPass()));
+ MPM.addPass(ProfileVerifierPass());
// Add any relevant output pass at the end of the pipeline.
switch (OK) {
diff --git a/llvm/utils/profcheck-xfail.txt b/llvm/utils/profcheck-xfail.txt
index 980f99687c4cc..3cde50de7d0c1 100644
--- a/llvm/utils/profcheck-xfail.txt
+++ b/llvm/utils/profcheck-xfail.txt
@@ -20,7 +20,6 @@ CodeGen/X86/masked_gather_scatter.ll
DebugInfo/AArch64/ir-outliner.ll
DebugInfo/assignment-tracking/X86/hotcoldsplit.ll
DebugInfo/Generic/block-asan.ll
-DebugInfo/X86/asan_debug_info.ll
LTO/X86/diagnostic-handler-remarks-with-hotness.ll
Other/optimization-remarks-auto.ll
Transforms/AtomicExpand/ARM/atomic-expansion-v7.ll
@@ -475,10 +474,6 @@ Transforms/LowerConstantIntrinsics/objectsize_basic.ll
Transforms/LowerGlobalDestructors/lower-global-dtors-existing-dos_handle.ll
Transforms/LowerGlobalDestructors/lower-global-dtors.ll
Transforms/LowerGlobalDestructors/non-literal-type.ll
-Transforms/LowerIFunc/ifunc-alias.ll
-Transforms/LowerIFunc/ifunc-nonsense-resolvers.ll
-Transforms/LowerIFunc/ifunc-program-addrspace.ll
-Transforms/LowerIFunc/lower-ifunc.ll
Transforms/LowerMatrixIntrinsics/data-layout-multiply-fused.ll
Transforms/LowerMatrixIntrinsics/multiply-fused-dominance.ll
Transforms/LowerMatrixIntrinsics/multiply-fused.ll
@@ -498,9 +493,6 @@ Transforms/LowerSwitch/do-not-handle-impossible-values.ll
Transforms/LowerSwitch/feature.ll
Transforms/LowerSwitch/fold-popular-case-to-unreachable-default.ll
Transforms/LowerSwitch/pr59316.ll
-Transforms/LowerTypeTests/cfi-coff-comdat-rename.ll
-Transforms/LowerTypeTests/function.ll
-Transforms/LowerTypeTests/function-weak.ll
Transforms/LowerTypeTests/import.ll
Transforms/LowerTypeTests/simple.ll
Transforms/MergeFunc/2011-02-08-RemoveEqual.ll
|
|
@llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesFunctions listed in Full diff: https://github.com/llvm/llvm-project/pull/170597.diff 8 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/ProfileVerify.h b/llvm/include/llvm/Transforms/Utils/ProfileVerify.h
index 5c9c44c23bc01..0ba611b574ed1 100644
--- a/llvm/include/llvm/Transforms/Utils/ProfileVerify.h
+++ b/llvm/include/llvm/Transforms/Utils/ProfileVerify.h
@@ -13,6 +13,7 @@
#ifndef LLVM_TRANSFORMS_UTILS_PROFILEVERIFY_H
#define LLVM_TRANSFORMS_UTILS_PROFILEVERIFY_H
+#include "llvm/ADT/DenseSet.h"
#include "llvm/IR/Analysis.h"
#include "llvm/IR/PassManager.h"
#include "llvm/Support/Compiler.h"
@@ -29,8 +30,11 @@ class ProfileInjectorPass : public PassInfoMixin<ProfileInjectorPass> {
/// in conjunction with the ProfileInjectorPass. MD_prof "unknown" is considered
/// valid (i.e. !{!"unknown"})
class ProfileVerifierPass : public PassInfoMixin<ProfileVerifierPass> {
+ DenseSet<const Function *> IgnoreList;
+ PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
+
public:
- LLVM_ABI PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
+ LLVM_ABI PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM);
};
} // namespace llvm
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index e9874ecd553ee..cf998f29ef38c 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -149,6 +149,7 @@ MODULE_PASS("print<ir2vec>", IR2VecPrinterPass(errs()))
MODULE_PASS("print<ir2vec-vocab>", IR2VecVocabPrinterPass(errs()))
MODULE_PASS("print<module-debuginfo>", ModuleDebugInfoPrinterPass(errs()))
MODULE_PASS("print<reg-usage>", PhysicalRegisterUsageInfoPrinterPass(errs()))
+MODULE_PASS("prof-verify", ProfileVerifierPass())
MODULE_PASS("pseudo-probe", SampleProfileProbePass(TM))
MODULE_PASS("pseudo-probe-update", PseudoProbeUpdatePass())
MODULE_PASS("recompute-globalsaa", RecomputeGlobalsAAPass())
@@ -526,7 +527,6 @@ FUNCTION_PASS("print<scev-division>", SCEVDivisionPrinterPass(errs()))
FUNCTION_PASS("print<stack-safety-local>", StackSafetyPrinterPass(errs()))
FUNCTION_PASS("print<uniformity>", UniformityInfoPrinterPass(errs()))
FUNCTION_PASS("prof-inject", ProfileInjectorPass())
-FUNCTION_PASS("prof-verify", ProfileVerifierPass())
FUNCTION_PASS("reassociate", ReassociatePass())
FUNCTION_PASS("redundant-dbg-inst-elim", RedundantDbgInstEliminationPass())
FUNCTION_PASS("replace-with-veclib", ReplaceWithVeclib())
diff --git a/llvm/lib/Transforms/Utils/ProfileVerify.cpp b/llvm/lib/Transforms/Utils/ProfileVerify.cpp
index ca6bd91b1f530..d9fb879bac74c 100644
--- a/llvm/lib/Transforms/Utils/ProfileVerify.cpp
+++ b/llvm/lib/Transforms/Utils/ProfileVerify.cpp
@@ -11,13 +11,19 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/Analysis/BranchProbabilityInfo.h"
#include "llvm/IR/Analysis.h"
+#include "llvm/IR/Constants.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Function.h"
+#include "llvm/IR/GlobalValue.h"
+#include "llvm/IR/GlobalVariable.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/PassManager.h"
#include "llvm/IR/ProfDataUtils.h"
#include "llvm/Support/BranchProbability.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/CommandLine.h"
using namespace llvm;
@@ -189,11 +195,39 @@ PreservedAnalyses ProfileInjectorPass::run(Function &F,
return PreservedAnalyses::none();
}
+PreservedAnalyses ProfileVerifierPass::run(Module &M,
+ ModuleAnalysisManager &MAM) {
+ auto PopulateIgnoreList = [&](StringRef GVName) {
+ if (const auto *CT = M.getGlobalVariable(GVName))
+ if (const auto *CA =
+ dyn_cast_if_present<ConstantArray>(CT->getInitializer()))
+ for (const auto &Elt : CA->operands())
+ if (const auto *CS = dyn_cast<ConstantStruct>(Elt))
+ if (CS->getNumOperands() >= 2 && CS->getOperand(1))
+ if (const auto *F = dyn_cast<Function>(
+ CS->getOperand(1)->stripPointerCasts()))
+ IgnoreList.insert(F);
+ };
+ PopulateIgnoreList("llvm.global_ctors");
+ PopulateIgnoreList("llvm.global_dtors");
+ struct Wrapper : PassInfoMixin<Wrapper> {
+ ProfileVerifierPass &PVP;
+ PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM) {
+ return PVP.run(F, FAM);
+ }
+ explicit Wrapper(ProfileVerifierPass &PVP) : PVP(PVP) {}
+ };
+
+ return createModuleToFunctionPassAdaptor(Wrapper(*this)).run(M, MAM);
+}
+
PreservedAnalyses ProfileVerifierPass::run(Function &F,
FunctionAnalysisManager &FAM) {
// skip purely asm functions
if (isAsmOnly(F))
return PreservedAnalyses::all();
+ if (IgnoreList.contains(&F))
+ return PreservedAnalyses::all();
const auto EntryCount = F.getEntryCount(/*AllowSynthetic=*/true);
if (!EntryCount) {
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify-known-cold.ll b/llvm/test/Transforms/PGOProfile/prof-verify-known-cold.ll
index 7875300006761..9fa2732fbc6b5 100644
--- a/llvm/test/Transforms/PGOProfile/prof-verify-known-cold.ll
+++ b/llvm/test/Transforms/PGOProfile/prof-verify-known-cold.ll
@@ -1,6 +1,6 @@
; Test prof-verify for functions explicitly marked as cold
-; RUN: opt -passes=prof-inject,prof-verify %s -o - 2>&1 | FileCheck %s
+; RUN: opt -passes=function(prof-inject),module(prof-verify) %s -o - 2>&1 | FileCheck %s
define void @foo(i32 %i) !prof !0 {
%c = icmp eq i32 %i, 0
diff --git a/llvm/test/Transforms/PGOProfile/prof-verify.ll b/llvm/test/Transforms/PGOProfile/prof-verify.ll
index 75d1e6a3db571..90a20b68ab23c 100644
--- a/llvm/test/Transforms/PGOProfile/prof-verify.ll
+++ b/llvm/test/Transforms/PGOProfile/prof-verify.ll
@@ -6,7 +6,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 -passes=function(prof-inject),module(prof-verify) %s --disable-output
; RUN: opt -enable-profcheck %s -S -o - | FileCheck %s --check-prefix=INJECT
define void @foo(i32 %i) !prof !0 {
diff --git a/llvm/test/Transforms/PGOProfile/profcheck-llvm.global_ctors.ll b/llvm/test/Transforms/PGOProfile/profcheck-llvm.global_ctors.ll
new file mode 100644
index 0000000000000..84c680fc158ac
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/profcheck-llvm.global_ctors.ll
@@ -0,0 +1,13 @@
+; RUN: opt -passes=prof-verify %s -o - 2>&1 | FileCheck %s
+@llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 0, ptr @ctor, ptr null }]
+@llvm.global_dtors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 0, ptr @dtor, ptr null }]
+
+define internal void @ctor() {
+ ret void
+}
+
+define internal void @dtor() {
+ ret void
+}
+
+; CHECK-NOT: Profile verification failed: function entry count missing
diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp
index 3209b652b44b4..eaa1d8f958a4d 100644
--- a/llvm/tools/opt/NewPMDriver.cpp
+++ b/llvm/tools/opt/NewPMDriver.cpp
@@ -520,7 +520,7 @@ bool llvm::runPassPipeline(
false, "", nullptr, DebugifyMode::OriginalDebugInfo,
&DebugInfoBeforePass, VerifyDIPreserveExport));
if (EnableProfcheck)
- MPM.addPass(createModuleToFunctionPassAdaptor(ProfileVerifierPass()));
+ MPM.addPass(ProfileVerifierPass());
// Add any relevant output pass at the end of the pipeline.
switch (OK) {
diff --git a/llvm/utils/profcheck-xfail.txt b/llvm/utils/profcheck-xfail.txt
index 980f99687c4cc..3cde50de7d0c1 100644
--- a/llvm/utils/profcheck-xfail.txt
+++ b/llvm/utils/profcheck-xfail.txt
@@ -20,7 +20,6 @@ CodeGen/X86/masked_gather_scatter.ll
DebugInfo/AArch64/ir-outliner.ll
DebugInfo/assignment-tracking/X86/hotcoldsplit.ll
DebugInfo/Generic/block-asan.ll
-DebugInfo/X86/asan_debug_info.ll
LTO/X86/diagnostic-handler-remarks-with-hotness.ll
Other/optimization-remarks-auto.ll
Transforms/AtomicExpand/ARM/atomic-expansion-v7.ll
@@ -475,10 +474,6 @@ Transforms/LowerConstantIntrinsics/objectsize_basic.ll
Transforms/LowerGlobalDestructors/lower-global-dtors-existing-dos_handle.ll
Transforms/LowerGlobalDestructors/lower-global-dtors.ll
Transforms/LowerGlobalDestructors/non-literal-type.ll
-Transforms/LowerIFunc/ifunc-alias.ll
-Transforms/LowerIFunc/ifunc-nonsense-resolvers.ll
-Transforms/LowerIFunc/ifunc-program-addrspace.ll
-Transforms/LowerIFunc/lower-ifunc.ll
Transforms/LowerMatrixIntrinsics/data-layout-multiply-fused.ll
Transforms/LowerMatrixIntrinsics/multiply-fused-dominance.ll
Transforms/LowerMatrixIntrinsics/multiply-fused.ll
@@ -498,9 +493,6 @@ Transforms/LowerSwitch/do-not-handle-impossible-values.ll
Transforms/LowerSwitch/feature.ll
Transforms/LowerSwitch/fold-popular-case-to-unreachable-default.ll
Transforms/LowerSwitch/pr59316.ll
-Transforms/LowerTypeTests/cfi-coff-comdat-rename.ll
-Transforms/LowerTypeTests/function.ll
-Transforms/LowerTypeTests/function-weak.ll
Transforms/LowerTypeTests/import.ll
Transforms/LowerTypeTests/simple.ll
Transforms/MergeFunc/2011-02-08-RemoveEqual.ll
|
boomanaiden154
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems reasonable enough to me. Some nits inline.
I wouldn't be opposed to also just fixing all the cases that make these synthetic functions at some point. Some people have used PGO in the past specifically to optimize for startup performance (although I believe they were using custom instrumentation and just doing code layout optimizations). It also doesn't look like much would have to be fixed.
Agreed that the performance on the table for long running apps isn't high though. Any possible benefits are likely to be code layout related which is better handled by a post link optimizer anyways.
| explicit Wrapper(ProfileVerifierPass &PVP) : PVP(PVP) {} | ||
| }; | ||
|
|
||
| return createModuleToFunctionPassAdaptor(Wrapper(*this)).run(M, MAM); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better to just iterate over the functions here rather than creating an adapter. We don't need any of the pass manager machinery because we're just looking at function metadata and we always return PreservedAnalyses::all().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"we always return PreserveAnalyses::all()" at the moment. The wrapper is very simple, and if iterating over functions we'd still need to get the FAM somehow, so lines of code-wise, approx the same thing, but the wrapper way is (imho) doing better separation of concerns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"we always return PreserveAnalyses::all()" at the moment.
Ah, true. Missed that this was profinject. I don't see any references to the FAM currently, and I don't think we actually need it given we're just looking at function metadata, but it is true that might change in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ProfileVerifierPass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we should never need to modify the IR and return something other than PreserveAnalyses::all()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...or we don't have to worry about those aspects at all 😀
f99aad3 to
9066e51
Compare
boomanaiden154
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable enough to me. If someone cares about this in the future, it's not hard to reenable verification.
|
We are seeing test failures on Not sure if it is related to the LIT internal shell. Could you revert the change and land the fix later? |
Functions listed in `llvm.global_ctors` or `llvm.global_dtors` aren't too interesting for performance. Passes like LowerTypeTests synthesize some of these, and, while we could add a "0" entry count explicitly, we can also just not bother verifying in the first place.

Functions listed in
llvm.global_ctorsorllvm.global_dtorsaren't too interesting for performance. Passes like LowerTypeTests synthesize some of these, and, while we could add a "0" entry count explicitly, we can also just not bother verifying in the first place.