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

[anemet]

Summary

This is obviously an interesting case because it may motivate code
restructuring or LTO.

Reporting this requires instantiation of ORE in the loop where the call
sites are first gathered. I've checked compile-time
overhead *with* -Rpass-with-hotness and the worst slow-down was 6% in
mcf and quickly tailing off. As before without -Rpass-with-hotness
there is no overhead.

Because this could be a pretty noisy diagnostics, it is currently
qualified as 'verbose'. As of this patch, 'verbose' diagnostics are
only emitted with -Rpass-with-hotness, i.e. when the output is expected
to be filtered.

[davidxl]

include/llvm/Analysis/OptimizationDiagnosticInfo.h

173

Is there a need for this wrapper?

lib/Transforms/IPO/Inliner.cpp

472

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?

[anemet]

include/llvm/Analysis/OptimizationDiagnosticInfo.h

173

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.

lib/Transforms/IPO/Inliner.cpp

472

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?

[anemet]

Address David's comments.

I've added a new verbosity paramater to the ORE APIs and marked this new
inliner remark verbose. For now Verbose remarks are only emitted
if -Rpass-with-hotness is on, i.e. it's expected that the output would be
filtered.

[anemet]

Ping. @davidxl, does the new version address your concerns?

[davidxl]

include/llvm/Analysis/OptimizationDiagnosticInfo.h

99

Perhaps change Verbose to IsVerbose or IsMsgVerbose

// If IsVerbose is true, the message is considered verbose and will only be emitted when verbose output is turned on

lib/Transforms/IPO/Inliner.cpp

472

verbose output is fine as long as there are ways for tools to do post-processing to filter/order messages according to importance. This is future work of course.

test/Transforms/Inline/optimization-remarks.ll

2

bad formatting?

[anemet]

include/llvm/Analysis/OptimizationDiagnosticInfo.h

99

IsVerbose WFM, thanks!

test/Transforms/Inline/optimization-remarks.ll

2

Why? I just wrapped the long line.

I usually do it like this: multiple RUN lines, separated with \, continuation lines indented.

[anemet]

Rename Verbose to IsVerbose per David's comment.

[davidxl]

lgtm

include/llvm/Analysis/OptimizationDiagnosticInfo.h

public:
   }
 
   /// Emit an optimization-missed message.
   ///
   /// \p PassName is the name of the pass emitting the message. If
   /// -Rpass-missed= is given and the name matches the regular expression in
   /// -Rpass, then the remark will be emitted.  \p DLoc is the debug location
   /// where the diagnostic is generated. \p V is the IR Value that identifies
-  /// the code region. \p Msg is the message string to use.
+  /// the code region. \p Msg is the message string to use.  If \p IsVerbose is
+  /// true, the message is considered verbose and will only be emitted when
+  /// verbose output is turned on.
   void emitOptimizationRemarkMissed(const char *PassName, const DebugLoc &DLoc,
-                                    const Value *V, const Twine &Msg);
+                                    const Value *V, const Twine &Msg,
+                                    bool IsVerbose = false);
 
   /// \brief Same as above but derives the IR Value for the code region and the
   /// debug location from the Loop parameter \p L.
   void emitOptimizationRemarkMissed(const char *PassName, Loop *L,
-                                    const Twine &Msg);
+                                    const Twine &Msg, bool IsVerbose = false);
 
   /// \brief Same as above but derives the debug location and the code region
   /// from the debug location and the basic block of \p Inst, respectively.
   void emitOptimizationRemarkMissed(const char *PassName, Instruction *Inst,
-                                    const Twine &Msg) {
+                                    const Twine &Msg, bool IsVerbose = false) {
     emitOptimizationRemarkMissed(PassName, Inst->getDebugLoc(),
-                                 Inst->getParent(), Msg);
+                                 Inst->getParent(), Msg, IsVerbose);
   }
 
   /// Emit an optimization analysis remark message.
   ///
   /// \p PassName is the name of the pass emitting the message. If
   /// -Rpass-analysis= is given and \p PassName matches the regular expression
   /// in -Rpass, then the remark will be emitted. \p DLoc is the debug location
   /// where the diagnostic is generated. \p V is the IR Value that identifies
-  /// the code region. \p Msg is the message string to use.
+  /// the code region. \p Msg is the message string to use. If \p IsVerbose is
+  /// true, the message is considered verbose and will only be emitted when
+  /// verbose output is turned on.
   void emitOptimizationRemarkAnalysis(const char *PassName,
                                       const DebugLoc &DLoc, const Value *V,
-                                      const Twine &Msg);
+                                      const Twine &Msg, bool IsVerbose = false);
 
   /// \brief Same as above but derives the IR Value for the code region and the
   /// debug location from the Loop parameter \p L.
   void emitOptimizationRemarkAnalysis(const char *PassName, Loop *L,
-                                      const Twine &Msg);
+                                      const Twine &Msg, bool IsVerbose = false);
 
   /// \brief Same as above but derives the debug location and the code region
   /// from the debug location and the basic block of \p Inst, respectively.
   void emitOptimizationRemarkAnalysis(const char *PassName, Instruction *Inst,
-                                      const Twine &Msg) {
+                                      const Twine &Msg,
+                                      bool IsVerbose = false) {
     emitOptimizationRemarkAnalysis(PassName, Inst->getDebugLoc(),
-                                   Inst->getParent(), Msg);
+                                   Inst->getParent(), Msg, IsVerbose);
+  }
+
+  /// \brief This variant allows specifying what should be emitted for missed
+  /// and analysis remarks in one call.
+  ///
+  /// \p PassName is the name of the pass emitting the message. If
+  /// -Rpass-missed= is given and \p PassName matches the regular expression, \p
+  /// MsgForMissedRemark is emitted.
+  ///
+  /// If -Rpass-analysis= is given and \p PassName matches the regular
+  /// expression, \p MsgForAnalysisRemark is emitted.
+  ///
+  /// The debug location and the code region is derived from \p Inst. If \p
+  /// IsVerbose is true, the message is considered verbose and will only be
+  /// emitted when verbose output is turned on.
+  void emitOptimizationRemarkMissedAndAnalysis(
+      const char *PassName, Instruction *Inst, const Twine &MsgForMissedRemark,
+      const Twine &MsgForAnalysisRemark, bool IsVerbose = false) {
+    emitOptimizationRemarkAnalysis(PassName, Inst, MsgForAnalysisRemark,
+                                   IsVerbose);
+    emitOptimizationRemarkMissed(PassName, Inst, MsgForMissedRemark, IsVerbose);
   }
 
   /// \brief Emit an optimization analysis remark related to floating-point
   /// non-commutativity.
   ///
   /// \p PassName is the name of the pass emitting the message. If
   /// -Rpass-analysis= is given and \p PassName matches the regular expression
   /// in -Rpass, then the remark will be emitted. \p Fn is the function
   /// triggering the remark, \p DLoc is the debug location where the diagnostic
   /// is generated.\p V is the IR Value that identifies the code region.  \p Msg
   /// is the message string to use.
   void emitOptimizationRemarkAnalysisFPCommute(const char *PassName,
                                                const DebugLoc &DLoc,
                                                const Value *V,
                                                const Twine &Msg);
 
   /// \brief Emit an optimization analysis remark related to pointer aliasing.
   ///
   /// \p PassName is the name of the pass emitting the message. If
   /// -Rpass-analysis= is given and \p PassName matches the regular expression
   /// in -Rpass, then the remark will be emitted. \p Fn is the function
   /// triggering the remark, \p DLoc is the debug location where the diagnostic
private:
 
   BlockFrequencyInfo *BFI;
 
   /// If we generate BFI on demand, we need to free it when ORE is freed.
   std::unique_ptr<BlockFrequencyInfo> OwnedBFI;
 
   Optional<uint64_t> computeHotness(const Value *V);
 
+  /// \brief Only allow verbose messages if we know we're filtering by hotness
+  /// (BFI is only set in this case).
+  bool shouldEmitVerbose() { return BFI != nullptr; }
+
   OptimizationRemarkEmitter(const OptimizationRemarkEmitter &) = delete;
   void operator=(const OptimizationRemarkEmitter &) = delete;
 };
 
 /// OptimizationRemarkEmitter legacy analysis pass
 ///
 /// Note that this pass shouldn't generally be marked as preserved by other
 /// passes.  It's holding onto BFI, so if the pass does not preserve BFI, BFI

lib/Analysis/OptimizationDiagnosticInfo.cpp

 void OptimizationRemarkEmitter::emitOptimizationRemark(const char *PassName,
                                                        Loop *L,
                                                        const Twine &Msg) {
   emitOptimizationRemark(PassName, L->getStartLoc(), L->getHeader(), Msg);
 }
 
 void OptimizationRemarkEmitter::emitOptimizationRemarkMissed(
     const char *PassName, const DebugLoc &DLoc, const Value *V,
-    const Twine &Msg) {
+    const Twine &Msg, bool IsVerbose) {
   LLVMContext &Ctx = F->getContext();
+  if (!IsVerbose || shouldEmitVerbose())
     Ctx.diagnose(DiagnosticInfoOptimizationRemarkMissed(PassName, *F, DLoc, Msg,
                                                         computeHotness(V)));
 }
 
 void OptimizationRemarkEmitter::emitOptimizationRemarkMissed(
-    const char *PassName, Loop *L, const Twine &Msg) {
-  emitOptimizationRemarkMissed(PassName, L->getStartLoc(), L->getHeader(), Msg);
+    const char *PassName, Loop *L, const Twine &Msg, bool IsVerbose) {
+  emitOptimizationRemarkMissed(PassName, L->getStartLoc(), L->getHeader(), Msg,
+                               IsVerbose);
 }
 
 void OptimizationRemarkEmitter::emitOptimizationRemarkAnalysis(
     const char *PassName, const DebugLoc &DLoc, const Value *V,
-    const Twine &Msg) {
+    const Twine &Msg, bool IsVerbose) {
   LLVMContext &Ctx = F->getContext();
-  Ctx.diagnose(DiagnosticInfoOptimizationRemarkAnalysis(PassName, *F, DLoc, Msg,
-                                                        computeHotness(V)));
+  if (!IsVerbose || shouldEmitVerbose())
+    Ctx.diagnose(DiagnosticInfoOptimizationRemarkAnalysis(
+        PassName, *F, DLoc, Msg, computeHotness(V)));
 }
 
 void OptimizationRemarkEmitter::emitOptimizationRemarkAnalysis(
-    const char *PassName, Loop *L, const Twine &Msg) {
+    const char *PassName, Loop *L, const Twine &Msg, bool IsVerbose) {
   emitOptimizationRemarkAnalysis(PassName, L->getStartLoc(), L->getHeader(),
-                                 Msg);
+                                 Msg, IsVerbose);
 }
 
 void OptimizationRemarkEmitter::emitOptimizationRemarkAnalysisFPCommute(
     const char *PassName, const DebugLoc &DLoc, const Value *V,
     const Twine &Msg) {
   LLVMContext &Ctx = F->getContext();
   Ctx.diagnose(DiagnosticInfoOptimizationRemarkAnalysisFPCommute(
       PassName, *F, DLoc, Msg, computeHotness(V)));

lib/Transforms/IPO/Inliner.cpp

inlineCallsImpl(CallGraphSCC &SCC, CallGraph &CG,
   // When inlining a callee produces new call sites, we want to keep track of
   // the fact that they were inlined from the callee.  This allows us to avoid
   // infinite inlining in some obscure cases.  To represent this, we use an
   // index into the InlineHistory vector.
   SmallVector<std::pair<Function *, int>, 8> InlineHistory;
 
   for (CallGraphNode *Node : SCC) {
     Function *F = Node->getFunction();
-    if (!F)
+    if (!F || F->isDeclaration())
       continue;
 
+    OptimizationRemarkEmitter ORE(F);
     for (BasicBlock &BB : *F)
       for (Instruction &I : BB) {
         CallSite CS(cast<Value>(&I));
         // If this isn't a call, or it is a call to an intrinsic, it can
         // never be inlined.
         if (!CS || isa<IntrinsicInst>(I))
           continue;
 
         // If this is a direct call to an external function, we can never inline
         // it.  If it is an indirect call, inlining may resolve it to be a
         // direct call, so we keep it.
         if (Function *Callee = CS.getCalledFunction())
-          if (Callee->isDeclaration())
+          if (Callee->isDeclaration()) {
+            ORE.emitOptimizationRemarkMissedAndAnalysis(
+                DEBUG_TYPE, &I,
+                Twine(Callee->getName()) + " will not be inlined into " +
+                    CS.getCaller()->getName(),
+                Twine("definition of ") + Callee->getName() +
+                    " is not available",
+                /*Verbose=*/true);
             continue;
+          }
 
         CallSites.push_back(std::make_pair(CS, -1));
       }
   }
 
   DEBUG(dbgs() << ": " << CallSites.size() << " call sites.\n");
 
   // If there are no calls in this function, exit early.

test/Transforms/Inline/optimization-remarks.ll

-; RUN: opt < %s -inline -pass-remarks=inline -pass-remarks-missed=inline -pass-remarks-analysis=inline -S 2>&1 | FileCheck %s
-
+; RUN: opt < %s -inline -pass-remarks=inline -pass-remarks-missed=inline \
+; RUN:       -pass-remarks-analysis=inline -S 2>&1 | \
+; RUN:       FileCheck -check-prefix=CHECK -check-prefix=NO_HOTNESS %s
+; RUN: opt < %s -inline -pass-remarks=inline -pass-remarks-missed=inline \
+; RUN:       -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 | \
+; RUN:       FileCheck -check-prefix=CHECK -check-prefix=HOTNESS %s
+
+; HOTNESS: definition of fox is not available
+; HOTNESS: fox will not be inlined into bar
+; NO_HOTNESS-NOT: definition of fox is not available
+; NO_HOTNESS-NOT: fox will not be inlined into bar
 ; CHECK: foo should always be inlined (cost=always)
 ; CHECK: foo inlined into bar
 ; CHECK: foz should never be inlined (cost=never)
 ; CHECK: foz will not be inlined into bar
 
 ; Function Attrs: alwaysinline nounwind uwtable
 define i32 @foo(i32 %x, i32 %y) #0 {
 entry:
entry:
   store i32 %y, i32* %y.addr, align 4
   %0 = load i32, i32* %x.addr, align 4
   %1 = load i32, i32* %y.addr, align 4
   %mul = mul nsw i32 %0, %1
   %conv = sitofp i32 %mul to float
   ret float %conv
 }
 
+declare i32 @fox()
+
 ; Function Attrs: nounwind uwtable
 define i32 @bar(i32 %j) #2 {
 entry:
   %j.addr = alloca i32, align 4
   store i32 %j, i32* %j.addr, align 4
   %0 = load i32, i32* %j.addr, align 4
   %1 = load i32, i32* %j.addr, align 4
   %sub = sub nsw i32 %1, 2
   %call = call i32 @foo(i32 %0, i32 %sub)
   %conv = sitofp i32 %call to float
   %2 = load i32, i32* %j.addr, align 4
   %sub1 = sub nsw i32 %2, 2
   %3 = load i32, i32* %j.addr, align 4
   %call2 = call float @foz(i32 %sub1, i32 %3)
   %mul = fmul float %conv, %call2
   %conv3 = fptosi float %mul to i32
-  ret i32 %conv3
+  %call3 = call i32 @fox()
+  %add = add i32 %conv3, %call 
+  ret i32 %add
 }
 
 attributes #0 = { alwaysinline nounwind uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
 attributes #1 = { noinline nounwind uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
 attributes #2 = { nounwind uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
 
 !llvm.ident = !{!0}
 
 !0 = !{!"clang version 3.5.0 "}