-
Notifications
You must be signed in to change notification settings - Fork 14.3k
InlineCostAnnotationPrinter: Fix constructing random TargetTransformInfo #133637
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
InlineCostAnnotationPrinter: Fix constructing random TargetTransformInfo #133637
Conversation
Query the correct TTI for the current target instead of constructing some random default one. Also query the pass manager for ProfileSummaryInfo.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
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.
Was introduced in https://reviews.llvm.org/D81743
@llvm/pr-subscribers-llvm-analysis Author: Matt Arsenault (arsenm) ChangesQuery the correct TTI for the current target instead of constructing Full diff: https://github.com/llvm/llvm-project/pull/133637.diff 1 Files Affected:
diff --git a/llvm/lib/Analysis/InlineCost.cpp b/llvm/lib/Analysis/InlineCost.cpp
index e42b2bd82cf2e..9f193b610328b 100644
--- a/llvm/lib/Analysis/InlineCost.cpp
+++ b/llvm/lib/Analysis/InlineCost.cpp
@@ -3295,9 +3295,12 @@ InlineCostAnnotationPrinterPass::run(Function &F,
[&](Function &F) -> AssumptionCache & {
return FAM.getResult<AssumptionAnalysis>(F);
};
- Module *M = F.getParent();
- ProfileSummaryInfo PSI(*M);
- TargetTransformInfo TTI(M->getDataLayout());
+
+ auto &MAMProxy = FAM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
+ ProfileSummaryInfo *PSI =
+ MAMProxy.getCachedResult<ProfileSummaryAnalysis>(*F.getParent());
+ const TargetTransformInfo &TTI = FAM.getResult<TargetIRAnalysis>(F);
+
// FIXME: Redesign the usage of InlineParams to expand the scope of this pass.
// In the current implementation, the type of InlineParams doesn't matter as
// the pass serves only for verification of inliner's decisions.
@@ -3312,7 +3315,7 @@ InlineCostAnnotationPrinterPass::run(Function &F,
continue;
OptimizationRemarkEmitter ORE(CalledFunction);
InlineCostCallAnalyzer ICCA(*CalledFunction, *CB, Params, TTI,
- GetAssumptionCache, nullptr, nullptr, &PSI,
+ GetAssumptionCache, nullptr, nullptr, PSI,
&ORE);
ICCA.analyze();
OS << " Analyzing call of " << CalledFunction->getName()
|
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.
Lgtm, but can the commit message say this is about the printer pass - i.e. this should have no effect on inlining cost itself?
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/198/builds/3270 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/23/builds/8918 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/199/builds/2472 Here is the relevant piece of the build log for the reference
|
…nfo (llvm#133637) Query the correct TTI for the current target instead of constructing some random default one. Also query the pass manager for ProfileSummaryInfo. This should only change the printing, not the actual result.
Query the correct TTI for the current target instead of constructing
some random default one. Also query the pass manager for ProfileSummaryInfo.
This should only change the printing, not the actual result.