This patch introduces InlineCostAnnotationPrinterPass - a pass that allows us to use the functionality of InlineCostAnnotationWriter in others than debug builds. This pass performs InlineCostAnalysis on each function that is being called from the given one and then dumps the result, thus activating InlineCostAnnotationWriter.
The main purpose of this pass is to verify the inliner's decisions made in InlineCost. This pass provides functionality to see inliner's logic.
Future revisions will add new functionality to InlineCostAnnotationWriter.
This patch is the second part of D81016.
Details
Diff Detail
Event Timeline
A question to @mtrofin:
I have changed the creation of InlineParams from
const InlineParams Params = llvm::getInlineParams();
which invokes the creation of InlineParams with DefaultThreshold to
const InlineParams Params = llvm::getInlineParams(InlineThreshold);
Is this correct?
I think it depends on what the goal of the pass is. If it is to capture what costs (pre- any inlining) you'd see during a run of InlinerPass, the question is which run. See PassBuilder.cpp. There are different InlineParams used, for instance, see addPGOInstrPasses, buildInlinerPipeline, and buildLTODefaultPipeline.
I suppose you could pass the InlineParameters as a ctor parameter to the pass?
Changed the creation of InlineParams back to default params and added a "to-do" comment.
For the purposes of this pass, the idea of which is to give access for non-debug builds to tests that use debug functionality, the InlineParams can be taken as default. My suggestion is to leave the extension of the pass to parametrization with different InlineParams as a "to-do" for now.
I apologize, maybe it's just me - but could you please elaborate the usage scenario, I don't fully follow. Here's what I understand and don't understand so far: in a non-Debug build (say, Release), a developer would run this pass (how?) and then use the results - but how, if the parameters are different from what clang used?
Otherwise the patch is fine, but I think it'd be valuable to capture the intended usage for maintainability - so others can understand why default params are fine there, for instance (or if not, how to correctly change the code, etc).
llvm/lib/Analysis/InlineCost.cpp | ||
---|---|---|
2525 | nit: I think it's "FIXME: Redesign the usage <...>" |
- Refactored the comment
For now, my view of the pass is to be able to tell what optimizations are seen and predicted by InlineCost. E.g. is a follow-up patch D81024 which enables printing of constants to which the instruction will be simplified. For now, if you've written a simplification for InlineCost that constant-folds some sequence of instructions, you have no way of writing test for it except by setting the inline threshold so low, so the fact of simplification will inline the method. This pass provides an easy way to see such cases directly. Also, the fact of simplification itself is independent of InlineParams, so the pass would work as expected.
Ah, OK! This description helps a lot, thank you!
If that's the case, could you update the patch description and the pass documentation to specify this is currently for tests, and the FIXME - basically, the FIXME is only if we want to scope this to more than tests.
- Changed the comments to better reflect the usage of the pass
- Changed the summary of the differential
lgtm
1 more nit - in the description "given one and them dumps the result": s/them/then
llvm/lib/Analysis/InlineCost.cpp | ||
---|---|---|
2527 | nit: the sentence doesn't finish |
nit: I think it's "FIXME: Redesign the usage <...>"