Page MenuHomePhabricator

Add support for missed and analysis optimization remarks.
ClosedPublic

Authored by dnovillo on May 8 2014, 3:14 PM.

Details

Summary

This adds two new diagnostics: -pass-remarks-missed and
-pass-remarks-analysis. They take the same values as -pass-remarks but
are intended to be triggered in different contexts.

-pass-remarks-missed is used by LLVMContext::emitOptimizationRemarkMissed,
which passes call when they tried to apply a transformation but
couldn't.

-pass-remarks-analysis is used by LLVMContext::emitOptimizationRemarkAnalysis,
which passes call when they want to inform the user about analysis
results.

The patch also adds support in the inliner for the two new remarks and a
test case.

Diff Detail

Repository
rL LLVM

Event Timeline

dnovillo updated this revision to Diff 9230.May 8 2014, 3:14 PM
dnovillo retitled this revision from to Add support for missed and analysis optimization remarks..
dnovillo updated this object.
dnovillo edited the test plan for this revision. (Show Details)
dnovillo added a reviewer: qcolombet.
dnovillo added a subscriber: Unknown Object (MLST).
qcolombet edited edge metadata.May 14 2014, 1:51 PM

Hi Diego,

This overall LGTM.
I have one concern though.
The LLVMContext begins to have a lot of its API dedicated to diagnostic report. All those helper functions calls LLVMContext::diagnose and I would prefer to have just this specific method (emitError being the exception) than having a profusion of such helpers in that class.
I think it would make sense to have those helper functions in the DiagnosticInfo header. That way it would be better self contained.
This is my 2 cents.

What do you think?

Thanks,
-Quentin

dnovillo updated this revision to Diff 9664.May 21 2014, 1:49 PM
dnovillo edited edge metadata.
  • Move emitOptimizationRemark* functions to llvm namespace.
qcolombet accepted this revision.May 21 2014, 3:45 PM
qcolombet edited edge metadata.

Hi Diego,

Thanks for the diff, I was wondering where some of the changes came from :).

This LGTM.

One remark, would it be possible to move the PassRemarkXXX options and related implementation in the DiagnosticInfo implementation too?
I.e., stuff related to static cl::opt<PassRemarksOpt, etc.

If it is possible, you can fix that in a subsequent commit, if it is not I can live with that :).

Thanks,
-Quentin

This revision is now accepted and ready to land.May 21 2014, 3:45 PM
dnovillo closed this revision.May 22 2014, 7:27 AM
dnovillo updated this revision to Diff 9697.

Closed by commit rL209442 (authored by @dnovillo).