Diff Detail
Event Timeline
| include/llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h | ||
|---|---|---|
| 82 | Need to clarify with either comment or name that this is the number of instructions recorded in the summary (i.e. it may have changed by this point which is why we don't recompute here). | |
| lib/Transforms/Utils/ImportedFunctionsInliningStatistics.cpp | ||
| 28 | Looks like the standard way to get an integer metadata is using mdconst::extract. E.g. from Value.cpp: if (MDNode *MD = LI->getMetadata(LLVMContext::MD_align)) {
ConstantInt *CI = mdconst::extract<ConstantInt>(MD->getOperand(0));
Align = CI->getLimitedValue();
} | |
| 139 | "#instructions per summary"? | |
There is no motivation in the description so it is unclear to me right now. Since the information is already in the summary, having the statistic with a function name already enables you to retrieve this from the combined index.
Ultimately it is not scalable to duplicate every information contained by the summary as metadata as you need them.
Right, I was originally thinking about suggesting an alternative which is to pass the index into the inliner so the info could simply be queried directly out of the index. However, it makes dumping these stats via something like "llvm-link -thinlto-action=import | opt -O2" impossible unless we pass the combined index in to opt as well, for passing to the inliner.
Could be a new optional cl::opt for ImportedFunctionsInliningStatistics providing the path to the index?
Otherwise it can also be that the printed statistics have to be passed to a third party tool to produce a report (with a flow similar to how code-coverage reporting works somehow).
Would be nice to avoid requiring a third-party tool to munge the stats if possible.
The suggestion of passing the index would work, but would require re-parsing the index when we already have it in the case where we came through the compiler (and not opt). Using the in-memory copy when already available would require a couple of changes:
- Stop gating the enabling of the function import pass in the pass manager based on the ModuleSummary in the pass manager being non-null (add a separate bool for that), then always set the ModuleSummary on the pass manager and pass it into the inliner
- In the opt tool, change the existing module-summary option to emit-module-summary, and add a new module-summary that takes an existing summary index file, reads it, and passes it into the inliner when it is created.
Need to clarify with either comment or name that this is the number of instructions recorded in the summary (i.e. it may have changed by this point which is why we don't recompute here).