[PATCH] D36054: Emit only A Single Opt Remark When Inlining

[Adam Nemet via Phabricator via llvm-commits]

anemet added inline comments.


================
Comment at: lib/Transforms/IPO/Inliner.cpp:879-882
+      InlineCost IC = shouldInline(CS, GetInlineCost, ORE);
       // Check whether we want to inline this callsite.
-      if (!shouldInline(CS, GetInlineCost, ORE))
+      if (!IC)
         continue;
----------------
lenary wrote:
> anemet wrote:
> > Isn't this the path via the new PM?  Don't you need to emit the remarks for the successful inlining here as well?
> > 
> > Should probably add a test for this if I am right ;).
> Ok, I didn't understand why there were two implementations, one for each pass makes more sense.
> 
> I need to fix that bug with the path option, which might need to land first, so that tests start failing. 
> I need to fix that bug with the path option, which might need to land first, so that tests start failing.

SGTM.



================
Comment at: lib/Transforms/IPO/Inliner.cpp:353
                  << ", Call: " << *CS.getInstruction() << "\n");
-    ORE.emit(OptimizationRemarkAnalysis(DEBUG_TYPE, "AlwaysInline", Call)
-             << NV("Callee", Callee)
-             << " should always be inlined (cost=always)");
-    return true;
+    return Optional<InlineCost>(IC);
   }
----------------
Don't we have a ctor that converts from T?  I.e. can't you just say return IC?


================
Comment at: lib/Transforms/IPO/Inliner.cpp:363
              << " because it should never be inlined (cost=never)");
-    return false;
+    return Optional<InlineCost>();
   }
----------------
return None?


================
Comment at: lib/Transforms/IPO/Inliner.cpp:579-580
 
-        // Report the inline decision.
-        ORE.emit(OptimizationRemark(DEBUG_TYPE, "Inlined", DLoc, Block)
-                 << NV("Callee", Callee) << " inlined into "
-                 << NV("Caller", Caller));
+        InlineCost IC = OIC.getValue();
+        if (IC.isAlways())
+          ORE.emit(OptimizationRemark(DEBUG_TYPE, "AlwaysInline", DLoc, Block)
----------------
How about: OIC->isAlways()


https://reviews.llvm.org/D36054