This is an archive of the discontinued LLVM Phabricator instance.

[PartialInlining] Add Optimization Remark Support
ClosedPublic

Authored by davidxl on Apr 21 2017, 7:56 PM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

davidxl created this revision.Apr 21 2017, 7:56 PM
davide added inline comments.Apr 22 2017, 11:38 AM
lib/Transforms/IPO/PartialInlining.cpp
32 ↗(On Diff #96271)

Any reason to change the DEBUG_TYPE ?

70–72 ↗(On Diff #96271)

Feel free to commit this separately now (before the review)

davidxl added inline comments.Apr 22 2017, 12:29 PM
lib/Transforms/IPO/PartialInlining.cpp
32 ↗(On Diff #96271)

This is used in -Rpass=<pass_name> for consistency. 'partialinlining' does not look like a friendly name to use.

davide requested changes to this revision.Apr 22 2017, 4:07 PM
davide added a subscriber: simon.f.whittaker.

It looks like you're missing the PM glue for requiring the analysis.
AU.addRequired<OptimizationRemarkEmitterWrapperPass>();, et similia (slightly annoying as we have to do that twice for the current and the future pass manager).

This revision now requires changes to proceed.Apr 22 2017, 4:08 PM
hfinkel added inline comments.
lib/Transforms/IPO/PartialInlining.cpp
32 ↗(On Diff #96290)

Please use partial-inlining (we almost always use dashes, not underscores, in these names).

The way it is used, it does not require any analysis pass (on demand ).

davidxl updated this revision to Diff 96292.Apr 22 2017, 8:30 PM
davidxl edited edge metadata.

Use dash instead of underscore.

davide accepted this revision.Apr 22 2017, 8:31 PM

LGTM

This revision is now accepted and ready to land.Apr 22 2017, 8:31 PM
This revision was automatically updated to reflect the committed changes.