[PATCH] D99302: [InlineCost] Make cost-benefit decision explicit
David Li via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 24 15:21:59 PDT 2021
davidxl added inline comments.
================
Comment at: llvm/lib/Analysis/InlineCost.cpp:2618
// Check if there was a reason to force inlining or no inlining.
+ if (ShouldInline.isSuccess() && CA.wasDecidedByCostBenefit())
+ return InlineCost::getAlways("benefit over cost");
----------------
wenlei wrote:
> davidxl wrote:
> > Should this be folded into line 2622?
> We wanted any positive cost-benefit decision to be distinguishable from a regular cost-threshold decision. This is because sample loader inlining looks at cost, and compare against its own threshold (`SampleProfileLoader::shouldInlineCandidate`). But we want cost-benefit decision to take precedence there too. I can give the three flat if some structure..
>
> Perhaps it'd be good to make negative cost-benefit decision distinguishable too, by using never. An alternative is to add a field in InlineCost indicating a decision is made through cost-benefit analysis.
what I meant is re-organize your change a little:
if (ShouldInline.isSuccess()) {
if (CA.wasDecidedByCostBenefit() {
...
}
if (CA.getCost() >= CA.getThreshold() ) {
}
}
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99302/new/
https://reviews.llvm.org/D99302
More information about the llvm-commits
mailing list