Page MenuHomePhabricator

Initial patch for inlining report
Needs RevisionPublic

Authored by rcox2 on Apr 21 2016, 5:37 PM.

Details

Summary

Initial patch for inlining report

This is an initial patch in a series of 3 patches. After applying this patch, an inlining report can be printed with the option -inline-report=X where X is a bit mask with values described in InlineReport.cpp. Inlining reasons are absent at this point.

Diff Detail

Event Timeline

rcox2 updated this revision to Diff 54598.Apr 21 2016, 5:37 PM
rcox2 retitled this revision from to Initial patch for inlining report .
rcox2 updated this object.
rcox2 added reviewers: reames, hfinkel, apilipenko.
rcox2 added subscribers: kbsmith1, llvm-commits.
qcolombet requested changes to this revision.Apr 22 2016, 10:08 AM
qcolombet added a reviewer: qcolombet.
qcolombet added inline comments.
include/llvm/Analysis/CallGraphReport.h
26

I believe we should have something of higher level like a Report class.
Other type of optimizations may report some stuff as well.
I was also wondering how different this is from the Remark mechanism and assuming we indeed need a new mechanism for reports, I think we should do something similar to the remarks.

This revision now requires changes to proceed.Apr 22 2016, 10:08 AM

Having a high level Report class would be useful. I talked to a few people at the Bay Area LLVM Social and they supported the idea of Report class into which all optimizaitons could report information.

The key difference between that and the remark mechanism is that the remark mechanism provides a way to give a "blow by blow" description of what optimizations have or haven't happened in time sequence. An optimization report (of which the inline report would be a first part) organizes the information in a more useful hierarchical and analyzable form.

Most applications that I look at have on the order of 10s of thousands of inlining decisions. Simply having them printed out in sequence, as is done with the optimization remarks, does not yield the info in a form that guides the user to act on it.

.

apilipenko edited edge metadata.Apr 26 2016, 10:33 AM

Uploading diff with full context would ease the reviewer's job (see http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)

hfinkel edited edge metadata.Apr 26 2016, 8:02 PM

Having a high level Report class would be useful. I talked to a few people at the Bay Area LLVM Social and they supported the idea of Report class into which all optimizaitons could report information.

The key difference between that and the remark mechanism is that the remark mechanism provides a way to give a "blow by blow" description of what optimizations have or haven't happened in time sequence. An optimization report (of which the inline report would be a first part) organizes the information in a more useful hierarchical and analyzable form.

Most applications that I look at have on the order of 10s of thousands of inlining decisions. Simply having them printed out in sequence, as is done with the optimization remarks, does not yield the info in a form that guides the user to act on it.

It is true that the current remark system prints out a "blow by blow" description, but the general infrastructure is not limited to that -- the frontend handler can do whatever it wishes. Furthermore, the entire remark system (DiagnosticInfo) was designed to allow the information to flow through the frontend so that various embedded analysis and display tools might intercept it. LLVM provides default handlers and printers. We should follow the same pattern here.

InliningInformation (collected during inlining) -> Set to Frontend (via the DiagnosticInfo class system) ->
  Frontend collects information  -- by default, it collects the information and prints it at the end into a report file [LLVM provides a default printer that all frontends can use if desired (all diagnostics and remarks can print themselves using some DiagnosticPrinter class)].

In short, the desired state is not necessrily too far removed from what you have, but it needs to be refactored a bit to fit into the infrastructure we have (and that we designed to be flexible enough to serve a wide variety of use cases)

Having a high level Report class would be useful. I talked to a few people at the Bay Area LLVM Social and they supported the idea of Report class into which all optimizaitons could report information.

The key difference between that and the remark mechanism is that the remark mechanism provides a way to give a "blow by blow" description of what optimizations have or haven't happened in time sequence. An optimization report (of which the inline report would be a first part) organizes the information in a more useful hierarchical and analyzable form.

Most applications that I look at have on the order of 10s of thousands of inlining decisions. Simply having them printed out in sequence, as is done with the optimization remarks, does not yield the info in a form that guides the user to act on it.

It is true that the current remark system prints out a "blow by blow" description, but the general infrastructure is not limited to that -- the frontend handler can do whatever it wishes. Furthermore, the entire remark system (DiagnosticInfo) was designed to allow the information to flow through the frontend so that various embedded analysis and display tools might intercept it. LLVM provides default handlers and printers. We should follow the same pattern here.

InliningInformation (collected during inlining) -> Set to Frontend (via the DiagnosticInfo class system) ->
  Frontend collects information  -- by default, it collects the information and prints it at the end into a report file [LLVM provides a default printer that all frontends can use if desired (all diagnostics and remarks can print themselves using some DiagnosticPrinter class)].

In short, the desired state is not necessrily too far removed from what you have, but it needs to be refactored a bit to fit into the infrastructure we have (and that we designed to be flexible enough to serve a wide variety of use cases)

Please see also D19678, as an example of intercepting the remarks in the frontend and doing something other than just printing them. This is not really "right" because the relevant remarks are not yet proper subclasses, but we can fix that up in parallel. Also feel free to help with that review.

Hal,

I’ve been looking at the patch that you posted for the annotated source listing to get an idea of how you would like the inlining report to conform to the diagnostic mechanism which you have already implemented.

It seems to me that you would like the information needed to construct the inlining report to be passed back to the diagnostic mechanism via function calls like:

emitOptimizationRemark()

when the inlining occurs, and

emitOptimizationRemarkMissed()

when the inlining does not occur.

Then, the report could be emitted in a manner similar to how you called OptReportAction::GenerateReportFile() for the annotated source listing.

Am I on the right track?

To do so, I would need to pass down more information than is currently being passed down. I would need at least something that identifies the callsite either being inlined or not inlined, and, when reasons are added, the reason (or reasons) the inlining did or didn’t happen.

Would I just add another parameter pointer to a class variable to do that?

  • Thanks, Robert Cox

Hal,

I’ve been looking at the patch that you posted for the annotated source listing to get an idea of how you would like the inlining report to conform to the diagnostic mechanism which you have already implemented.

It seems to me that you would like the information needed to construct the inlining report to be passed back to the diagnostic mechanism via function calls like:

emitOptimizationRemark()

when the inlining occurs, and

emitOptimizationRemarkMissed()

when the inlining does not occur.

Yes (although you should use a proper subclass to store all of the relevant information in member variables). The other thing to track is the conversation about serialization of these things into YAML for consumption by external tools.

Then, the report could be emitted in a manner similar to how you called OptReportAction::GenerateReportFile() for the annotated source listing.

Yes, although it sounds like the chosen path is to dump all of the information into YAML files, and then have the actual report generation in external tools that can consume those files, and the sources, to produce reports.

Am I on the right track?

To do so, I would need to pass down more information than is currently being passed down. I would need at least something that identifies the callsite either being inlined or not inlined, and, when reasons are added, the reason (or reasons) the inlining did or didn’t happen.

You mean into those functions? Yes, you'd need to use different functions (or directly create the remark classes and then directly call 'diagnose' with that class).

Would I just add another parameter pointer to a class variable to do that?

  • Thanks, Robert Cox
rcox2 added a comment.Jun 2 2016, 4:19 PM

Hal,

I’ve been looking at the patch that you posted for the annotated source listing to get an idea of how you would like the inlining report to conform to the diagnostic mechanism which you have already implemented.

It seems to me that you would like the information needed to construct the inlining report to be passed back to the diagnostic mechanism via function calls like:

emitOptimizationRemark()

when the inlining occurs, and

emitOptimizationRemarkMissed()

when the inlining does not occur.

Yes (although you should use a proper subclass to store all of the relevant information in member variables). The other thing to track is the conversation about serialization of these things into YAML for consumption by external tools.

OK, so, for example, in DiagnosticInfo.h I see:

class DiagnosticInfoOptimizationRemark : public DiagnosticInfoOptimizationBase {

...

}

and in DiagnosticInfo.cpp, I see:

void llvm::emitOptimizationRemark(LLVMContext &Ctx, const char *PassName,

                                const Function &Fn, const DebugLoc &DLoc,
                                const Twine &Msg) {
Ctx.diagnose(DiagnosticInfoOptimizationRemark(PassName, Fn, DLoc, Msg));

}

and in Inliner.cpp., I see a call:

emitOptimizationRemark(
     CallerCtx, DEBUG_TYPE, *Caller, DLoc,
     Twine(Callee->getName() + " inlined into " + Caller->getName()));

To extend this, I would declare a subclass:

class DiagnosticInfoInlineOptimizationRemark: public DiagnosticInfoOptimizationRemark {

...

}

Add a definition:

void llvm::emitOptimizationInlineRemark(LLVMContext &Ctx,

                                const CallSite& CS, const DebugLoc &DLoc,
                                const Twine &Msg) {
Ctx.diagnose(DiagnosticInfoOptimizationRemark(CS, DLoc, Msg));

}

and then replace the call to emitOptimizationRemark() in Inliner.cpp with a call to emitOptimizationInlineRemark();

Let me know if I have this right.

Then, the report could be emitted in a manner similar to how you called OptReportAction::GenerateReportFile() for the annotated source listing.

Yes, although it sounds like the chosen path is to dump all of the information into YAML files, and then have the actual report generation in external tools that can consume those files, and the sources, to produce reports.

So, I'm trying to understand if we would go through the process of generating a YAML even when we need YAML for an external tool or not.

What are your thoughts?

Am I on the right track?

To do so, I would need to pass down more information than is currently being passed down. I would need at least something that identifies the callsite either being inlined or not inlined, and, when reasons are added, the reason (or reasons) the inlining did or didn’t happen.

You mean into those functions? Yes, you'd need to use different functions (or directly create the remark classes and then directly call 'diagnose' with that class).

Would I just add another parameter pointer to a class variable to do that?

  • Thanks, Robert Cox

Hal,

I’ve been looking at the patch that you posted for the annotated source listing to get an idea of how you would like the inlining report to conform to the diagnostic mechanism which you have already implemented.

It seems to me that you would like the information needed to construct the inlining report to be passed back to the diagnostic mechanism via function calls like:

emitOptimizationRemark()

when the inlining occurs, and

emitOptimizationRemarkMissed()

when the inlining does not occur.

Yes (although you should use a proper subclass to store all of the relevant information in member variables). The other thing to track is the conversation about serialization of these things into YAML for consumption by external tools.

OK, so, for example, in DiagnosticInfo.h I see:

class DiagnosticInfoOptimizationRemark : public DiagnosticInfoOptimizationBase {

...

}

and in DiagnosticInfo.cpp, I see:

void llvm::emitOptimizationRemark(LLVMContext &Ctx, const char *PassName,

                                const Function &Fn, const DebugLoc &DLoc,
                                const Twine &Msg) {
Ctx.diagnose(DiagnosticInfoOptimizationRemark(PassName, Fn, DLoc, Msg));

}

and in Inliner.cpp., I see a call:

emitOptimizationRemark(
     CallerCtx, DEBUG_TYPE, *Caller, DLoc,
     Twine(Callee->getName() + " inlined into " + Caller->getName()));

To extend this, I would declare a subclass:

class DiagnosticInfoInlineOptimizationRemark: public DiagnosticInfoOptimizationRemark {

...

}

Add a definition:

void llvm::emitOptimizationInlineRemark(LLVMContext &Ctx,

                                const CallSite& CS, const DebugLoc &DLoc,
                                const Twine &Msg) {
Ctx.diagnose(DiagnosticInfoOptimizationRemark(CS, DLoc, Msg));

}

and then replace the call to emitOptimizationRemark() in Inliner.cpp with a call to emitOptimizationInlineRemark();

Yes, that sounds about right. I don't think we need emitOptimizationInlineRemark(), the inliner should probably just call diagnose directly with its newly-created DiagnosticInfoInlineOptimizationRemark object. The DiagnosticInfoInlineOptimizationRemark will likely need a bunch of constructor parameters anyway to capture all of the various inlining details.

Let me know if I have this right.

Then, the report could be emitted in a manner similar to how you called OptReportAction::GenerateReportFile() for the annotated source listing.

Yes, although it sounds like the chosen path is to dump all of the information into YAML files, and then have the actual report generation in external tools that can consume those files, and the sources, to produce reports.

So, I'm trying to understand if we would go through the process of generating a YAML even when we need YAML for an external tool or not.

What are your thoughts?

I don't understand your question. We'll want some general infrastructure for serializing all of the remarks into YAML, although we don't have it yet.

Thanks again, Hal

Am I on the right track?

To do so, I would need to pass down more information than is currently being passed down. I would need at least something that identifies the callsite either being inlined or not inlined, and, when reasons are added, the reason (or reasons) the inlining did or didn’t happen.

You mean into those functions? Yes, you'd need to use different functions (or directly create the remark classes and then directly call 'diagnose' with that class).

Would I just add another parameter pointer to a class variable to do that?

  • Thanks, Robert Cox