<div dir="ltr">It'd be great to mention in the commit message why a patch is being reverted (& usually helpful to mention the revision number of the original commit) to help folks understand what's happening with the patch/why.</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Aug 24, 2018 at 9:40 AM David Bolvansky via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: xbolva00<br>
Date: Fri Aug 24 09:39:41 2018<br>
New Revision: 340619<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=340619&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=340619&view=rev</a><br>
Log:<br>
Revert [Inliner] Attribute callsites with inline remarks<br>
<br>
Removed:<br>
    llvm/trunk/test/Transforms/Inline/inline-remark.ll<br>
Modified:<br>
    llvm/trunk/lib/Transforms/IPO/Inliner.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/Inliner.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=340619&r1=340618&r2=340619&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/Inliner.cpp?rev=340619&r1=340618&r2=340619&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Fri Aug 24 09:39:41 2018<br>
@@ -113,14 +113,6 @@ static cl::opt<InlinerFunctionImportStat<br>
                           "printing of statistics for each inlined function")),<br>
     cl::Hidden, cl::desc("Enable inliner stats for imported functions"));<br>
<br>
-/// Flag to add inline messages as callsite attributes 'inline-remark'.<br>
-static cl::opt<bool><br>
-    InlineRemarkAttribute("inline-remark-attribute", cl::init(false),<br>
-                          cl::Hidden,<br>
-                          cl::desc("Enable adding inline-remark attribute to"<br>
-                                   " callsites processed by inliner but decided"<br>
-                                   " to be not inlined"));<br>
-<br>
 LegacyInlinerBase::LegacyInlinerBase(char &ID) : CallGraphSCCPass(ID) {}<br>
<br>
 LegacyInlinerBase::LegacyInlinerBase(char &ID, bool InsertLifetime)<br>
@@ -272,7 +264,7 @@ static void mergeInlinedArrayAllocas(<br>
 /// available from other functions inlined into the caller.  If we are able to<br>
 /// inline this call site we attempt to reuse already available allocas or add<br>
 /// any new allocas to the set if not possible.<br>
-static InlineResult InlineCallIfPossible(<br>
+static bool InlineCallIfPossible(<br>
     CallSite CS, InlineFunctionInfo &IFI,<br>
     InlinedArrayAllocasTy &InlinedArrayAllocas, int InlineHistory,<br>
     bool InsertLifetime, function_ref<AAResults &(Function &)> &AARGetter,<br>
@@ -398,11 +390,13 @@ RemarkT &operator<<(RemarkT &&R, const I<br>
   return R;<br>
 }<br>
<br>
+#ifndef NDEBUG<br>
 static std::string inlineCostStr(const InlineCost &IC) {<br>
   std::stringstream Remark;<br>
   Remark << IC;<br>
   return Remark.str();<br>
 }<br>
+#endif<br>
<br>
 /// Return the cost only if the inliner should attempt to inline at the given<br>
 /// CallSite. If we return the cost, we will emit an optimisation remark later<br>
@@ -508,14 +502,6 @@ static void emit_inlined_into(Optimizati<br>
   });<br>
 }<br>
<br>
-static void setInlineRemark(CallSite &CS, StringRef message) {<br>
-  if (!InlineRemarkAttribute)<br>
-    return;<br>
-<br>
-  Attribute attr = Attribute::get(CS->getContext(), "inline-remark", message);<br>
-  CS.addAttribute(AttributeList::FunctionIndex, attr);<br>
-}<br>
-<br>
 static bool<br>
 inlineCallsImpl(CallGraphSCC &SCC, CallGraph &CG,<br>
                 std::function<AssumptionCache &(Function &)> GetAssumptionCache,<br>
@@ -565,7 +551,6 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG<br>
           if (Callee->isDeclaration()) {<br>
             using namespace ore;<br>
<br>
-            setInlineRemark(CS, "unavailable definition");<br>
             ORE.emit([&]() {<br>
               return OptimizationRemarkMissed(DEBUG_TYPE, "NoDefinition", &I)<br>
                      << NV("Callee", Callee) << " will not be inlined into "<br>
@@ -629,10 +614,8 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG<br>
         // infinitely inline.<br>
         InlineHistoryID = CallSites[CSi].second;<br>
         if (InlineHistoryID != -1 &&<br>
-            InlineHistoryIncludes(Callee, InlineHistoryID, InlineHistory)) {<br>
-          setInlineRemark(CS, "recursive");<br>
+            InlineHistoryIncludes(Callee, InlineHistoryID, InlineHistory))<br>
           continue;<br>
-        }<br>
       }<br>
<br>
       // FIXME for new PM: because of the old PM we currently generate ORE and<br>
@@ -643,15 +626,7 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG<br>
       Optional<InlineCost> OIC = shouldInline(CS, GetInlineCost, ORE);<br>
       // If the policy determines that we should inline this function,<br>
       // delete the call instead.<br>
-      if (!OIC.hasValue()) {<br>
-        setInlineRemark(CS, "deferred");<br>
-        continue;<br>
-      }<br>
-<br>
-      if (!OIC.getValue()) {<br>
-        // shouldInline() call returned a negative inline cost that explains<br>
-        // why this callsite should not be inlined.<br>
-        setInlineRemark(CS, inlineCostStr(*OIC));<br>
+      if (!OIC || !*OIC) {<br>
         continue;<br>
       }<br>
<br>
@@ -662,7 +637,6 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG<br>
       if (IsTriviallyDead) {<br>
         LLVM_DEBUG(dbgs() << "    -> Deleting dead call: " << *Instr << "\n");<br>
         // Update the call graph by deleting the edge from Callee to Caller.<br>
-        setInlineRemark(CS, "trivially dead");<br>
         CG[Caller]->removeCallEdgeFor(CS);<br>
         Instr->eraseFromParent();<br>
         ++NumCallsDeleted;<br>
@@ -678,7 +652,6 @@ inlineCallsImpl(CallGraphSCC &SCC, CallG<br>
             CS, InlineInfo, InlinedArrayAllocas, InlineHistoryID,<br>
             InsertLifetime, AARGetter, ImportedFunctionsStats);<br>
         if (!IR) {<br>
-          setInlineRemark(CS, std::string(IR) + "; " + inlineCostStr(*OIC));<br>
           ORE.emit([&]() {<br>
             return OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc,<br>
                                             Block)<br>
@@ -921,7 +894,6 @@ PreservedAnalyses InlinerPass::run(LazyC<br>
             Calls.push_back({CS, -1});<br>
           else if (!isa<IntrinsicInst>(I)) {<br>
             using namespace ore;<br>
-            setInlineRemark(CS, "unavailable definition");<br>
             ORE.emit([&]() {<br>
               return OptimizationRemarkMissed(DEBUG_TYPE, "NoDefinition", &I)<br>
                      << NV("Callee", Callee) << " will not be inlined into "<br>
@@ -965,10 +937,8 @@ PreservedAnalyses InlinerPass::run(LazyC<br>
     LazyCallGraph::Node &N = *CG.lookup(F);<br>
     if (CG.lookupSCC(N) != C)<br>
       continue;<br>
-    if (F.hasFnAttribute(Attribute::OptimizeNone)) {<br>
-      setInlineRemark(Calls[i].first, "optnone attribute");<br>
+    if (F.hasFnAttribute(Attribute::OptimizeNone))<br>
       continue;<br>
-    }<br>
<br>
     LLVM_DEBUG(dbgs() << "Inlining calls in: " << F.getName() << "\n");<br>
<br>
@@ -1012,10 +982,8 @@ PreservedAnalyses InlinerPass::run(LazyC<br>
       Function &Callee = *CS.getCalledFunction();<br>
<br>
       if (InlineHistoryID != -1 &&<br>
-          InlineHistoryIncludes(&Callee, InlineHistoryID, InlineHistory)) {<br>
-        setInlineRemark(CS, "recursive");<br>
+          InlineHistoryIncludes(&Callee, InlineHistoryID, InlineHistory))<br>
         continue;<br>
-      }<br>
<br>
       // Check if this inlining may repeat breaking an SCC apart that has<br>
       // already been split once before. In that case, inlining here may<br>
@@ -1027,23 +995,13 @@ PreservedAnalyses InlinerPass::run(LazyC<br>
         LLVM_DEBUG(dbgs() << "Skipping inlining internal SCC edge from a node "<br>
                              "previously split out of this SCC by inlining: "<br>
                           << F.getName() << " -> " << Callee.getName() << "\n");<br>
-        setInlineRemark(CS, "recursive SCC split");<br>
         continue;<br>
       }<br>
<br>
       Optional<InlineCost> OIC = shouldInline(CS, GetInlineCost, ORE);<br>
       // Check whether we want to inline this callsite.<br>
-      if (!OIC.hasValue()) {<br>
-        setInlineRemark(CS, "deferred");<br>
+      if (!OIC || !*OIC)<br>
         continue;<br>
-      }<br>
-<br>
-      if (!OIC.getValue()) {<br>
-        // shouldInline() call returned a negative inline cost that explains<br>
-        // why this callsite should not be inlined.<br>
-        setInlineRemark(CS, inlineCostStr(*OIC));<br>
-        continue;<br>
-      }<br>
<br>
       // Setup the data structure used to plumb customization into the<br>
       // `InlineFunction` routine.<br>
@@ -1060,7 +1018,6 @@ PreservedAnalyses InlinerPass::run(LazyC<br>
<br>
       InlineResult IR = InlineFunction(CS, IFI);<br>
       if (!IR) {<br>
-        setInlineRemark(CS, std::string(IR) + "; " + inlineCostStr(*OIC));<br>
         ORE.emit([&]() {<br>
           return OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc, Block)<br>
                  << NV("Callee", &Callee) << " will not be inlined into "<br>
<br>
Removed: llvm/trunk/test/Transforms/Inline/inline-remark.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline-remark.ll?rev=340618&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/inline-remark.ll?rev=340618&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/Inline/inline-remark.ll (original)<br>
+++ llvm/trunk/test/Transforms/Inline/inline-remark.ll (removed)<br>
@@ -1,96 +0,0 @@<br>
-; RUN: opt < %s -inline -inline-remark-attribute --inline-threshold=-2 -S | FileCheck %s<br>
-<br>
-; Test that the inliner adds inline remark attributes to non-inlined callsites.<br>
-<br>
-define void @foo() {<br>
-  call void @bar(i1 true)<br>
-  ret void<br>
-}<br>
-<br>
-define void @bar(i1 %p) {<br>
-  br i1 %p, label %bb1, label %bb2<br>
-<br>
-bb1:<br>
-  call void @foo()<br>
-  ret void<br>
-<br>
-bb2:<br>
-  call void @bar(i1 true)<br>
-  ret void<br>
-}<br>
-<br>
-;; Test 1 - Add different inline remarks to similar callsites.<br>
-define void @test1() {<br>
-; CHECK-LABEL: @test1<br>
-; CHECK-NEXT: call void @bar(i1 true) [[ATTR1:#[0-9]+]]<br>
-; CHECK-NEXT: call void @bar(i1 false) [[ATTR2:#[0-9]+]]<br>
-  call void @bar(i1 true)<br>
-  call void @bar(i1 false)<br>
-  ret void<br>
-}<br>
-<br>
-define void @noop() {<br>
-  ret void<br>
-}<br>
-<br>
-;; Test 2 - Printed InlineResult messages are followed by InlineCost.<br>
-define void @test2(i8*) {<br>
-; CHECK-LABEL: @test2<br>
-; CHECK-NEXT: call void @noop() [[ATTR3:#[0-9]+]] [ "CUSTOM_OPERAND_BUNDLE"() ]<br>
-; CHECK-NEXT: ret void<br>
-  call void @noop() ; extepected to be inlined<br>
-  call void @noop() [ "CUSTOM_OPERAND_BUNDLE"() ] ; cannot be inlined because of unsupported operand bundle<br>
-  ret void<br>
-}<br>
-<br>
-; CHECK: attributes [[ATTR1]] = { "inline-remark"="(cost=-5, threshold=-6)" }<br>
-; CHECK: attributes [[ATTR2]] = { "inline-remark"="(cost=never): recursive" }<br>
-; CHECK: attributes [[ATTR3]] = { "inline-remark"="unsupported operand bundle; (cost={{.*}}, threshold={{.*}})" }<br>
-; RUN: opt < %s -inline -inline-remark-attribute --inline-threshold=-2 -S | FileCheck %s<br>
-<br>
-; Test that the inliner adds inline remark attributes to non-inlined callsites.<br>
-<br>
-define void @foo() {<br>
-  call void @bar(i1 true)<br>
-  ret void<br>
-}<br>
-<br>
-define void @bar(i1 %p) {<br>
-  br i1 %p, label %bb1, label %bb2<br>
-<br>
-bb1:<br>
-  call void @foo()<br>
-  ret void<br>
-<br>
-bb2:<br>
-  call void @bar(i1 true)<br>
-  ret void<br>
-}<br>
-<br>
-;; Test 1 - Add different inline remarks to similar callsites.<br>
-define void @test1() {<br>
-; CHECK-LABEL: @test1<br>
-; CHECK-NEXT: call void @bar(i1 true) [[ATTR1:#[0-9]+]]<br>
-; CHECK-NEXT: call void @bar(i1 false) [[ATTR2:#[0-9]+]]<br>
-  call void @bar(i1 true)<br>
-  call void @bar(i1 false)<br>
-  ret void<br>
-}<br>
-<br>
-define void @noop() {<br>
-  ret void<br>
-}<br>
-<br>
-;; Test 2 - Printed InlineResult messages are followed by InlineCost.<br>
-define void @test2(i8*) {<br>
-; CHECK-LABEL: @test2<br>
-; CHECK-NEXT: call void @noop() [[ATTR3:#[0-9]+]] [ "CUSTOM_OPERAND_BUNDLE"() ]<br>
-; CHECK-NEXT: ret void<br>
-  call void @noop() ; extepected to be inlined<br>
-  call void @noop() [ "CUSTOM_OPERAND_BUNDLE"() ] ; cannot be inlined because of unsupported operand bundle<br>
-  ret void<br>
-}<br>
-<br>
-; CHECK: attributes [[ATTR1]] = { "inline-remark"="(cost=-5, threshold=-6)" }<br>
-; CHECK: attributes [[ATTR2]] = { "inline-remark"="(cost=never): recursive" }<br>
-; CHECK: attributes [[ATTR3]] = { "inline-remark"="unsupported operand bundle; (cost={{.*}}, threshold={{.*}})" }<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>