Emit only A Single Opt Remark When Inlining

Summary:
This updates the Inliner to only add a single Optimization
Remark when Inlining, rather than an Analysis Remark and an
Optimization Remark.

Fixes https://bugs.llvm.org/show_bug.cgi?id=33786

Reviewers: anemet, davidxl, chandlerc

Reviewed By: anemet

Subscribers: haicheng, fhahn, mehdi_amini, dblaikie, llvm-commits, eraman

Differential Revision: https://reviews.llvm.org/D36054

llvm-svn: 311349

include/llvm/Analysis/InlineCost.h

@@ -105,6 +105,12 @@ public:
     return Cost;
   }
 
+  /// \brief Get the threshold against which the cost was computed
+  int getThreshold() const {
+    assert(isVariable() && "Invalid access of InlineCost");
+    return Threshold;
+  }
+
   /// \brief Get the cost delta from the threshold for inlining.
   /// Only valid if the cost is of the variable kind. Returns a negative
   /// value if the cost is too high to inline.

lib/Transforms/IPO/Inliner.cpp

@@ -335,10 +335,12 @@ shouldBeDeferred(Function *Caller, CallSite CS, InlineCost IC,
   return false;
 }
 
-/// Return true if the inliner should attempt to inline at the given CallSite.
+/// Return the cost only if the inliner should attempt to inline at the given
-static bool shouldInline(CallSite CS,
+/// CallSite. If we return the cost, we will emit an optimisation remark later
-                         function_ref<InlineCost(CallSite CS)> GetInlineCost,
+/// using that cost, so we won't do so from this function.
-                         OptimizationRemarkEmitter &ORE) {
+static Optional<InlineCost>
+shouldInline(CallSite CS, function_ref<InlineCost(CallSite CS)> GetInlineCost,
+             OptimizationRemarkEmitter &ORE) {
   using namespace ore;
   InlineCost IC = GetInlineCost(CS);
   Instruction *Call = CS.getInstruction();
@@ -348,10 +350,7 @@ static bool shouldInline(CallSite CS,
   if (IC.isAlways()) {
     DEBUG(dbgs() << "    Inlining: cost=always"
                  << ", Call: " << *CS.getInstruction() << "\n");
-    ORE.emit(OptimizationRemarkAnalysis(DEBUG_TYPE, "AlwaysInline", Call)
+    return IC;
-             << NV("Callee", Callee)
-             << " should always be inlined (cost=always)");
-    return true;
   }
 
   if (IC.isNever()) {
@@ -361,19 +360,19 @@ static bool shouldInline(CallSite CS,
              << NV("Callee", Callee) << " not inlined into "
              << NV("Caller", Caller)
              << " because it should never be inlined (cost=never)");
-    return false;
+    return None;
   }
 
   if (!IC) {
     DEBUG(dbgs() << "    NOT Inlining: cost=" << IC.getCost()
-                 << ", thres=" << (IC.getCostDelta() + IC.getCost())
+                 << ", thres=" << IC.getThreshold()
                  << ", Call: " << *CS.getInstruction() << "\n");
     ORE.emit(OptimizationRemarkMissed(DEBUG_TYPE, "TooCostly", Call)
              << NV("Callee", Callee) << " not inlined into "
              << NV("Caller", Caller) << " because too costly to inline (cost="
-             << NV("Cost", IC.getCost()) << ", threshold="
+             << NV("Cost", IC.getCost())
-             << NV("Threshold", IC.getCostDelta() + IC.getCost()) << ")");
+             << ", threshold=" << NV("Threshold", IC.getThreshold()) << ")");
-    return false;
+    return None;
   }
 
   int TotalSecondaryCost = 0;
@@ -386,18 +385,16 @@ static bool shouldInline(CallSite CS,
              << "Not inlining. Cost of inlining " << NV("Callee", Callee)
              << " increases the cost of inlining " << NV("Caller", Caller)
              << " in other contexts");
-    return false;
+
+    // IC does not bool() to false, so get an InlineCost that will.
+    // This will not be inspected to make an error message.
+    return None;
   }
 
   DEBUG(dbgs() << "    Inlining: cost=" << IC.getCost()
-               << ", thres=" << (IC.getCostDelta() + IC.getCost())
+               << ", thres=" << IC.getThreshold()
                << ", Call: " << *CS.getInstruction() << '\n');
-  ORE.emit(OptimizationRemarkAnalysis(DEBUG_TYPE, "CanBeInlined", Call)
+  return IC;
-           << NV("Callee", Callee) << " can be inlined into "
-           << NV("Caller", Caller) << " with cost=" << NV("Cost", IC.getCost())
-           << " (threshold="
-           << NV("Threshold", IC.getCostDelta() + IC.getCost()) << ")");
-  return true;
 }
 
 /// Return true if the specified inline history ID
@@ -545,9 +542,10 @@ inlineCallsImpl(CallGraphSCC &SCC, CallGraph &CG,
       // just become a regular analysis dependency.
       OptimizationRemarkEmitter ORE(Caller);
 
+      Optional<InlineCost> OIC = shouldInline(CS, GetInlineCost, ORE);
       // If the policy determines that we should inline this function,
       // delete the call instead.
-      if (!shouldInline(CS, GetInlineCost, ORE))
+      if (!OIC)
         continue;
 
       // If this call site is dead and it is to a readonly function, we should
@@ -562,7 +560,7 @@ inlineCallsImpl(CallGraphSCC &SCC, CallGraph &CG,
         ++NumCallsDeleted;
       } else {
         // Get DebugLoc to report. CS will be invalid after Inliner.
-        DebugLoc DLoc = Instr->getDebugLoc();
+        DebugLoc DLoc = CS->getDebugLoc();
         BasicBlock *Block = CS.getParent();
 
         // Attempt to inline the function.
@@ -578,10 +576,17 @@ inlineCallsImpl(CallGraphSCC &SCC, CallGraph &CG,
         }
         ++NumInlined;
 
-        // Report the inline decision.
+        if (OIC->isAlways())
-        ORE.emit(OptimizationRemark(DEBUG_TYPE, "Inlined", DLoc, Block)
+          ORE.emit(OptimizationRemark(DEBUG_TYPE, "AlwaysInline", DLoc, Block)
-                 << NV("Callee", Callee) << " inlined into "
+                   << NV("Callee", Callee) << " inlined into "
-                 << NV("Caller", Caller));
+                   << NV("Caller", Caller) << " with cost=always");
+        else
+          ORE.emit(OptimizationRemark(DEBUG_TYPE, "Inlined", DLoc, Block)
+                   << NV("Callee", Callee) << " inlined into "
+                   << NV("Caller", Caller)
+                   << " with cost=" << NV("Cost", OIC->getCost())
+                   << " (threshold=" << NV("Threshold", OIC->getThreshold())
+                   << ")");
 
         // If inlining this function gave us any new call sites, throw them
         // onto our worklist to process.  They are useful inline candidates.
@@ -885,8 +890,9 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC,
         continue;
       }
 
+      Optional<InlineCost> OIC = shouldInline(CS, GetInlineCost, ORE);
       // Check whether we want to inline this callsite.
-      if (!shouldInline(CS, GetInlineCost, ORE))
+      if (!OIC)
         continue;
 
       // Setup the data structure used to plumb customization into the
@@ -896,11 +902,32 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC,
           &FAM.getResult<BlockFrequencyAnalysis>(*(CS.getCaller())),
           &FAM.getResult<BlockFrequencyAnalysis>(Callee));
 
-      if (!InlineFunction(CS, IFI))
+      // Get DebugLoc to report. CS will be invalid after Inliner.
+      DebugLoc DLoc = CS->getDebugLoc();
+      BasicBlock *Block = CS.getParent();
+
+      using namespace ore;
+      if (!InlineFunction(CS, IFI)) {
+        ORE.emit(
+            OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc, Block)
+            << NV("Callee", &Callee) << " will not be inlined into "
+            << NV("Caller", &F));
         continue;
+      }
       DidInline = true;
       InlinedCallees.insert(&Callee);
 
+      if (OIC->isAlways())
+        ORE.emit(OptimizationRemark(DEBUG_TYPE, "AlwaysInline", DLoc, Block)
+                 << NV("Callee", &Callee) << " inlined into "
+                 << NV("Caller", &F) << " with cost=always");
+      else
+        ORE.emit(
+            OptimizationRemark(DEBUG_TYPE, "Inlined", DLoc, Block)
+            << NV("Callee", &Callee) << " inlined into " << NV("Caller", &F)
+            << " with cost=" << NV("Cost", OIC->getCost())
+            << " (threshold=" << NV("Threshold", OIC->getThreshold()) << ")");
+
       // Add any new callsites to defined functions to the worklist.
       if (!IFI.InlinedCallSites.empty()) {
         int NewHistoryID = InlineHistory.size();

test/LTO/Resolution/X86/diagnostic-handler-remarks-with-hotness.ll

@@ -17,6 +17,11 @@
 ; YAML-NEXT:   - Callee:          tinkywinky
 ; YAML-NEXT:   - String:          ' inlined into '
 ; YAML-NEXT:   - Caller:          main
+; YAML-NEXT:   - String:          ' with cost='
+; YAML-NEXT:   - Cost:            '-15000'
+; YAML-NEXT:   - String:          ' (threshold='
+; YAML-NEXT:   - Threshold:       '337'
+; YAML-NEXT:   - String:          ')'
 ; YAML-NEXT: ...
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"

test/LTO/Resolution/X86/diagnostic-handler-remarks.ll

@@ -15,6 +15,11 @@
 ; YAML-NEXT:   - Callee:          tinkywinky
 ; YAML-NEXT:   - String:          ' inlined into '
 ; YAML-NEXT:   - Caller:          main
+; YAML-NEXT:   - String:          ' with cost='
+; YAML-NEXT:   - Cost:            '-15000'
+; YAML-NEXT:   - String:          ' (threshold='
+; YAML-NEXT:   - Threshold:       '337'
+; YAML-NEXT:   - String:          ')'
 ; YAML-NEXT: ...
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"

test/LTO/X86/diagnostic-handler-remarks-with-hotness.ll

@@ -17,6 +17,11 @@
 ; YAML-NEXT:   - Callee:          foo
 ; YAML-NEXT:   - String:          ' inlined into '
 ; YAML-NEXT:   - Caller:          main
+; YAML-NEXT:   - String:          ' with cost='
+; YAML-NEXT:   - Cost:            '-15000'
+; YAML-NEXT:   - String:          ' (threshold='
+; YAML-NEXT:   - Threshold:       '337'
+; YAML-NEXT:   - String:          ')'
 ; YAML-NEXT: ...
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"

test/LTO/X86/diagnostic-handler-remarks.ll

@@ -53,6 +53,11 @@
 ; YAML-NEXT:   - Callee:          foo
 ; YAML-NEXT:   - String:          ' inlined into '
 ; YAML-NEXT:   - Caller:          main
+; YAML-NEXT:   - String:          ' with cost='
+; YAML-NEXT:   - Cost:            '-15000'
+; YAML-NEXT:   - String:          ' (threshold='
+; YAML-NEXT:   - Threshold:       '337'
+; YAML-NEXT:   - String:          ')'
 ; YAML-NEXT: ...
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"

test/ThinLTO/X86/diagnostic-handler-remarks-with-hotness.ll

@@ -25,6 +25,11 @@
 ; YAML1-NEXT:   - Callee:          foo
 ; YAML1-NEXT:   - String:          ' inlined into '
 ; YAML1-NEXT:   - Caller:          main
+; YAML1-NEXT:   - String:          ' with cost='
+; YAML1-NEXT:   - Cost:            '-30'
+; YAML1-NEXT:   - String:          ' (threshold='
+; YAML1-NEXT:   - Threshold:       '337'
+; YAML1-NEXT:   - String:          ')'
 ; YAML1-NEXT: ...
 
 
@@ -38,6 +43,11 @@
 ; YAML2-NEXT:   - Callee:          bar
 ; YAML2-NEXT:   - String:          ' inlined into '
 ; YAML2-NEXT:   - Caller:          foo
+; YAML2-NEXT:   - String:          ' with cost='
+; YAML2-NEXT:   - Cost:            '-30'
+; YAML2-NEXT:   - String:          ' (threshold='
+; YAML2-NEXT:   - Threshold:       '337'
+; YAML2-NEXT:   - String:          ')'
 ; YAML2-NEXT: ...
 
 

test/ThinLTO/X86/diagnostic-handler-remarks.ll

@@ -22,6 +22,11 @@
 ; YAML1-NEXT:   - Callee:          foo
 ; YAML1-NEXT:   - String:          ' inlined into '
 ; YAML1-NEXT:   - Caller:          main
+; YAML1-NEXT:   - String:          ' with cost='
+; YAML1-NEXT:   - Cost:            '-30'
+; YAML1-NEXT:   - String:          ' (threshold='
+; YAML1-NEXT:   - Threshold:       '337'
+; YAML1-NEXT:   - String:          ')'
 ; YAML1-NEXT: ...
 
 
@@ -35,6 +40,11 @@
 ; YAML2-NEXT:   - Callee:          bar
 ; YAML2-NEXT:   - String:          ' inlined into '
 ; YAML2-NEXT:   - Caller:          foo
+; YAML2-NEXT:   - String:          ' with cost='
+; YAML2-NEXT:   - Cost:            '-30'
+; YAML2-NEXT:   - String:          ' (threshold='
+; YAML2-NEXT:   - Threshold:       '337'
+; YAML2-NEXT:   - String:          ')'
 ; YAML2-NEXT: ...
 
 

test/Transforms/Inline/optimization-remarks-passed-yaml.ll

@@ -3,6 +3,11 @@
 ; RUN:    -pass-remarks-with-hotness 2>&1 | FileCheck %s
 ; RUN: cat %t | FileCheck -check-prefix=YAML %s
 
+; RUN: opt < %s -S -passes=inline -pass-remarks-output=%t -pass-remarks=inline \
+; RUN:    -pass-remarks-missed=inline -pass-remarks-analysis=inline \
+; RUN:    -pass-remarks-with-hotness 2>&1 | FileCheck %s
+; RUN: cat %t | FileCheck -check-prefix=YAML %s
+
 ; Check the YAML file for inliner-generated passed and analysis remarks.  This
 ; is the input:
 
@@ -12,28 +17,9 @@
 ;  4       return foo();
 ;  5     }
 
-; CHECK:      remark: /tmp/s.c:4:10: foo can be inlined into bar with cost={{[0-9\-]+}} (threshold={{[0-9]+}}) (hotness: 30)
+; CHECK: remark: /tmp/s.c:4:10: foo inlined into bar with cost={{[0-9\-]+}} (threshold={{[0-9]+}}) (hotness: 30)
-; CHECK-NEXT: remark: /tmp/s.c:4:10: foo inlined into bar (hotness: 30)
 
-; YAML:      --- !Analysis
+; YAML:      --- !Passed
-; YAML-NEXT: Pass:            inline
-; YAML-NEXT: Name:            CanBeInlined
-; YAML-NEXT: DebugLoc:        { File: /tmp/s.c, Line: 4, Column: 10 }
-; YAML-NEXT: Function:        bar
-; YAML-NEXT: Hotness:         30
-; YAML-NEXT: Args:
-; YAML-NEXT:   - Callee: foo
-; YAML-NEXT:     DebugLoc:        { File: /tmp/s.c, Line: 1, Column: 0 }
-; YAML-NEXT:   - String: ' can be inlined into '
-; YAML-NEXT:   - Caller: bar
-; YAML-NEXT:     DebugLoc:        { File: /tmp/s.c, Line: 3, Column: 0 }
-; YAML-NEXT:   - String: ' with cost='
-; YAML-NEXT:   - Cost: '{{[0-9\-]+}}'
-; YAML-NEXT:   - String: ' (threshold='
-; YAML-NEXT:   - Threshold: '{{[0-9]+}}'
-; YAML-NEXT:   - String: ')'
-; YAML-NEXT: ...
-; YAML-NEXT: --- !Passed
 ; YAML-NEXT: Pass:            inline
 ; YAML-NEXT: Name:            Inlined
 ; YAML-NEXT: DebugLoc:        { File: /tmp/s.c, Line: 4, Column: 10 }
@@ -45,6 +31,11 @@
 ; YAML-NEXT:   - String: ' inlined into '
 ; YAML-NEXT:   - Caller: bar
 ; YAML-NEXT:     DebugLoc:        { File: /tmp/s.c, Line: 3, Column: 0 }
+; YAML-NEXT:   - String: ' with cost='
+; YAML-NEXT:   - Cost: '{{[0-9\-]+}}'
+; YAML-NEXT:   - String: ' (threshold='
+; YAML-NEXT:   - Threshold: '{{[0-9]+}}'
+; YAML-NEXT:   - String: ')'
 ; YAML-NEXT: ...
 
 ; ModuleID = '/tmp/s.c'

test/Transforms/Inline/optimization-remarks-with-hotness.ll

@@ -1,9 +1,11 @@
 ; 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 %s
+; RUN: opt < %s -passes=inline -pass-remarks=inline -pass-remarks-missed=inline \
+; RUN:     -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 \
+; RUN:     | FileCheck %s
 
-; CHECK: foo should always be inlined (cost=always) (hotness: 30)
+; CHECK: foo inlined into bar with cost=always (hotness: 30)
-; CHECK: foo inlined into bar (hotness: 30)
 ; CHECK: foz not inlined into bar because it should never be inlined (cost=never) (hotness: 30)
 
 ; Function Attrs: alwaysinline nounwind uwtable

test/Transforms/Inline/optimization-remarks-yaml.ll

@@ -17,6 +17,26 @@
 ; The remarks output file should be empty.
 ; RUN: test ! -s %t.threshold
 
+; NewPM:
+; RUN: opt < %s -S -passes=inline -pass-remarks-missed=inline \
+; RUN:     -pass-remarks-with-hotness -pass-remarks-hotness-threshold 15 \
+; RUN:     -pass-remarks-output=%t 2>&1 | FileCheck %s -check-prefix=CHECK_NEW
+; RUN: test ! -s %t
+; RUN: opt < %s -S -passes=inline -pass-remarks-with-hotness -pass-remarks-output=%t
+; RUN: test ! -s %t
+;
+; Verify that remarks that don't meet the hotness threshold are not output.
+; RUN: opt < %s -S -passes=inline -pass-remarks-missed=inline \
+; RUN:     -pass-remarks-with-hotness -pass-remarks-hotness-threshold 100 \
+; RUN:     -pass-remarks-output=%t.threshold 2>&1 | \
+; RUN:     FileCheck -check-prefix=THRESHOLD %s
+; RUN: test ! -s %t.threshold
+; RUN: opt < %s -S -passes=inline \
+; RUN:     -pass-remarks-with-hotness -pass-remarks-hotness-threshold 100 \
+; RUN:     -pass-remarks-output=%t.threshold
+; The remarks output file should be empty.
+; RUN: test ! -s %t.threshold
+
 ; Check the YAML file generated for inliner remarks for this program:
 ;
 ;   1  int foo();
@@ -59,6 +79,9 @@
 ; No remarks should be output, since none meet the threshold.
 ; THRESHOLD-NOT: remark
 
+; NewPM does not output this kind of "missed" remark.
+; CHECK_NEW-NOT: remark
+
 ; ModuleID = '/tmp/s.c'
 source_filename = "/tmp/s.c"
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"

test/Transforms/Inline/optimization-remarks.ll

@@ -5,10 +5,18 @@
 ; RUN:       -pass-remarks-analysis=inline -pass-remarks-with-hotness -S 2>&1 | \
 ; RUN:       FileCheck -check-prefix=CHECK -check-prefix=HOTNESS %s
 
+; RUN: opt < %s -passes=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 -passes=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_NEW %s
+
 ; HOTNESS: fox will not be inlined into bar because its definition is unavailable
 ; NO_HOTNESS-NOT: fox will not be inlined into bar because its definition is unavailable
-; CHECK: foo should always be inlined (cost=always)
+; NewPM's inliner does not emit the following remark:
-; CHECK: foo inlined into bar
+; HOTNESS_NEW-NOT: fox will not be inlined into bar because its definition is unavailable
+; CHECK: foo inlined into bar with cost=always
 ; CHECK: foz not inlined into bar because it should never be inlined (cost=never)
 
 ; Function Attrs: alwaysinline nounwind uwtable