[PATCH] D23415: [Inliner] Report when inlining fails because callee's def is unavailable

Adam Nemet via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 17 09:40:46 PDT 2016


anemet added inline comments.

================
Comment at: include/llvm/Analysis/OptimizationDiagnosticInfo.h:146
@@ +145,3 @@
+  /// The debug location and the code region is derived from \p Inst.
+  void emitOptimizationRemarkMissedAndAnalysis(
+      const char *PassName, Instruction *Inst, const Twine &MsgForMissedRemark,
----------------
davidxl wrote:
> Is there a need for this wrapper?
Yes, it should be a pretty common use case to report both the fact that an opt failed and the reason for it.  I am surprised we didn't need it so far (@hfinkel on IRC was actually surprised we didn't have such an API yet).

I guess one explanation that in the vectorizer analysis remarks are emitted inside the function checking legality along with the specific reasons while the missed-opt remark is emitted upon returning from the legality check function.

================
Comment at: lib/Transforms/IPO/Inliner.cpp:455
@@ +454,3 @@
+          if (Callee->isDeclaration()) {
+            ORE.emitOptimizationRemarkMissedAndAnalysis(
+                DEBUG_TYPE, &I,
----------------
davidxl wrote:
> This message can be useful for thinLTO compilation, but for O2, I fear it can be quite noisy.  Perhaps using another option to make it default?
Good point.

I think that eventually we will have to introduce a concept of accuracy/relevance/verbosity in these calls.  These APIs are likely to go through some fundamental changes because the plan is to be able to gather all these remarks via YAML for post-processing.

For now, let me just experiment only reporting these when -Rpass-with-hotness is on.  Then we know that we have a higher tolerance for noise because we're only looking at hot regions.  This requires extending the API with verbosity parameter or something.  @hfinkel, let me know if you have comments on this.  Again, this is likely to be temporary but it's good to start having this notion in these APIs.

It would also be good to exclude reporting declarations from system headers.  Any idea how to do that?


https://reviews.llvm.org/D23415





More information about the llvm-commits mailing list