[llvm] r290457 - [PM] Teach the always inlining test case to be much more strict about

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 23 15:33:36 PST 2016


Author: chandlerc
Date: Fri Dec 23 17:33:35 2016
New Revision: 290457

URL: http://llvm.org/viewvc/llvm-project?rev=290457&view=rev
Log:
[PM] Teach the always inlining test case to be much more strict about
whether functions are removed, and fix the new PM's always inliner to
actually pass this test.

Without this, the new PM's always inliner leaves all the functions
kicking around which won't work out very well given the semantics of
always inline.

Doing this really highlights how frustrating the current alwaysinline
semantic contract is though -- why can we put it on *external*
functions, etc?

Also I've added a number of tricky and interesting test cases for
removing functions with the always inliner. There is one remaining case
not handled -- fully removing comdats -- and I've left a FIXME about
this.

Modified:
    llvm/trunk/lib/Transforms/IPO/AlwaysInliner.cpp
    llvm/trunk/test/Transforms/Inline/always-inline.ll

Modified: llvm/trunk/lib/Transforms/IPO/AlwaysInliner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/AlwaysInliner.cpp?rev=290457&r1=290456&r2=290457&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/AlwaysInliner.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/AlwaysInliner.cpp Fri Dec 23 17:33:35 2016
@@ -38,6 +38,7 @@ PreservedAnalyses AlwaysInlinerPass::run
   InlineFunctionInfo IFI;
   SmallSetVector<CallSite, 16> Calls;
   bool Changed = false;
+  SmallVector<Function *, 16> InlinedFunctions;
   for (Function &F : M)
     if (!F.isDeclaration() && F.hasFnAttribute(Attribute::AlwaysInline) &&
         isInlineViable(F)) {
@@ -52,8 +53,22 @@ PreservedAnalyses AlwaysInlinerPass::run
         // FIXME: We really shouldn't be able to fail to inline at this point!
         // We should do something to log or check the inline failures here.
         Changed |= InlineFunction(CS, IFI);
+
+      // Remember to try and delete this function afterward. This both avoids
+      // re-walking the rest of the module and avoids dealing with any iterator
+      // invalidation issues while deleting functions.
+      InlinedFunctions.push_back(&F);
     }
 
+  // Now try to delete all the functions we inlined.
+  for (Function *InlinedF : InlinedFunctions) {
+    InlinedF->removeDeadConstantUsers();
+    // FIXME: We should use some utility to handle cases where we can
+    // completely remove the comdat.
+    if (InlinedF->isDefTriviallyDead() && !InlinedF->hasComdat())
+      M.getFunctionList().erase(InlinedF);
+  }
+
   return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
 }
 

Modified: llvm/trunk/test/Transforms/Inline/always-inline.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/always-inline.ll?rev=290457&r1=290456&r2=290457&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/Inline/always-inline.ll (original)
+++ llvm/trunk/test/Transforms/Inline/always-inline.ll Fri Dec 23 17:33:35 2016
@@ -11,7 +11,8 @@
 ; 'alwaysinline'.
 ; RUN: opt < %s -passes=always-inline -S | FileCheck %s --check-prefix=CHECK
 
-define i32 @inner1() alwaysinline {
+define internal i32 @inner1() alwaysinline {
+; CHECK-NOT: @inner1(
   ret i32 1
 }
 define i32 @outer1() {
@@ -23,13 +24,14 @@ define i32 @outer1() {
    ret i32 %r
 }
 
-; The always inliner can't DCE internal functions. PR2945
-; CHECK-LABEL: @pr2945(
+; The always inliner can't DCE arbitrary internal functions. PR2945
 define internal i32 @pr2945() nounwind {
+; CHECK-LABEL: @pr2945(
   ret i32 0
 }
 
 define internal void @inner2(i32 %N) alwaysinline {
+; CHECK-NOT: @inner2(
   %P = alloca i32, i32 %N
   ret void
 }
@@ -50,7 +52,9 @@ define void @outer2(i32 %N) {
 declare i32 @a() returns_twice
 declare i32 @b() returns_twice
 
-define i32 @inner3() alwaysinline {
+; Cannot alwaysinline when that would introduce a returns_twice call.
+define internal i32 @inner3() alwaysinline {
+; CHECK-LABEL: @inner3(
 entry:
   %call = call i32 @a() returns_twice
   %add = add nsw i32 1, %call
@@ -67,7 +71,8 @@ entry:
   ret i32 %add
 }
 
-define i32 @inner4() alwaysinline returns_twice {
+define internal i32 @inner4() alwaysinline returns_twice {
+; CHECK-NOT: @inner4(
 entry:
   %call = call i32 @b() returns_twice
   %add = add nsw i32 1, %call
@@ -85,7 +90,9 @@ entry:
   ret i32 %add
 }
 
-define i32 @inner5(i8* %addr) alwaysinline {
+; We can't inline this even though it has alwaysinline!
+define internal i32 @inner5(i8* %addr) alwaysinline {
+; CHECK-LABEL: @inner5(
 entry:
   indirectbr i8* %addr, [ label %one, label %two ]
 
@@ -106,7 +113,9 @@ define i32 @outer5(i32 %x) {
   ret i32 %call
 }
 
-define void @inner6(i32 %x) alwaysinline {
+; We alwaysinline a function that call itself recursively.
+define internal void @inner6(i32 %x) alwaysinline {
+; CHECK-LABEL: @inner6(
 entry:
   %icmp = icmp slt i32 %x, 0
   br i1 %icmp, label %return, label %bb
@@ -129,7 +138,9 @@ entry:
   ret void
 }
 
+; This is not an alwaysinline function and is actually external.
 define i32 @inner7() {
+; CHECK-LABEL: @inner7(
   ret i32 1
 }
 define i32 @outer7() {
@@ -141,7 +152,8 @@ define i32 @outer7() {
    ret i32 %r
 }
 
-define float* @inner8(float* nocapture align 128 %a) alwaysinline {
+define internal float* @inner8(float* nocapture align 128 %a) alwaysinline {
+; CHECK-NOT: @inner8(
   ret float* %a
 }
 define float @outer8(float* nocapture %a) {
@@ -153,3 +165,137 @@ define float @outer8(float* nocapture %a
   %f = load float, float* %inner_a, align 4
   ret float %f
 }
+
+
+; The 'inner9*' and 'outer9' functions are designed to check that we remove
+; a function that is inlined by the always inliner even when it is used by
+; a complex constant expression prior to being inlined.
+
+; The 'a' function gets used in a complex constant expression that, despite
+; being constant folded, means it isn't dead. As a consequence it shouldn't be
+; deleted. If it is, then the constant expression needs to become more complex
+; to accurately test this scenario.
+define internal void @inner9a(i1 %b) alwaysinline {
+; CHECK-LABEL: @inner9a(
+entry:
+  ret void
+}
+
+define internal void @inner9b(i1 %b) alwaysinline {
+; CHECK-NOT: @inner9b(
+entry:
+  ret void
+}
+
+declare void @dummy9(i1 %b)
+
+define void @outer9() {
+; CHECK-LABEL: @outer9(
+entry:
+  ; First we use @inner9a in a complex constant expression that may get folded
+  ; but won't get removed, and then we call it which will get inlined. Despite
+  ; this the function can't be deleted because of the constant expression
+  ; usage.
+  %sink = alloca i1
+  store volatile i1 icmp eq (i64 ptrtoint (void (i1)* @inner9a to i64), i64 ptrtoint(void (i1)* @dummy9 to i64)), i1* %sink
+; CHECK: store volatile
+  call void @inner9a(i1 false)
+; CHECK-NOT: call void @inner9a
+
+  ; Next we call @inner9b passing in a constant expression. This constant
+  ; expression will in fact be removed by inlining, so we should also be able
+  ; to delete the function.
+  call void @inner9b(i1 icmp eq (i64 ptrtoint (void (i1)* @inner9b to i64), i64 ptrtoint(void (i1)* @dummy9 to i64)))
+; CHECK-NOT: @inner9b
+
+  ret void
+; CHECK: ret void
+}
+
+; The 'inner10' and 'outer10' functions test a frustrating consquence of the
+; current 'alwaysinline' semantic model. Because such functions are allowed to
+; be external functions, it may be necessary to both inline all of their uses
+; and leave them in the final output. These tests can be removed if and when
+; we restrict alwaysinline further.
+define void @inner10() alwaysinline {
+; CHECK-LABEL: @inner10(
+entry:
+  ret void
+}
+
+define void @outer10() {
+; CHECK-LABEL: @outer10(
+entry:
+  call void @inner10()
+; CHECK-NOT: call void @inner10
+
+  ret void
+; CHECK: ret void
+}
+
+; The 'inner11' and 'outer11' functions test another dimension of non-internal
+; functions with alwaysinline. These functions use external linkages that we can
+; actually remove safely and so we should.
+define linkonce void @inner11a() alwaysinline {
+; CHECK-NOT: @inner11a(
+entry:
+  ret void
+}
+
+define available_externally void @inner11b() alwaysinline {
+; CHECK-NOT: @inner11b(
+entry:
+  ret void
+}
+
+define void @outer11() {
+; CHECK-LABEL: @outer11(
+entry:
+  call void @inner11a()
+  call void @inner11b()
+; CHECK-NOT: call void @inner11a
+; CHECK-NOT: call void @inner11b
+
+  ret void
+; CHECK: ret void
+}
+
+; The 'inner12' and 'outer12' functions test that we don't remove functions
+; which are part of a comdat group even if they otherwise seem dead.
+$comdat12 = comdat any
+
+define linkonce void @inner12() alwaysinline comdat($comdat12) {
+; CHECK-LABEL: @inner12(
+t :qentry:
+  ret void
+}
+
+define void @outer12() comdat($comdat12) {
+; CHECK-LABEL: @outer12(
+entry:
+  call void @inner12()
+; CHECK-NOT: call void @inner12
+
+  ret void
+; CHECK: ret void
+}
+
+; The 'inner12' and 'outer12' functions test that we don't remove functions
+; which are part of a comdat group even if they otherwise seem dead.
+$comdat12 = comdat any
+
+define linkonce void @inner12() alwaysinline comdat($comdat12) {
+; CHECK-LABEL: @inner12(
+entry:
+  ret void
+}
+
+define void @outer12() comdat($comdat12) {
+; CHECK-LABEL: @outer12(
+entry:
+  call void @inner12()
+; CHECK-NOT: call void @inner12
+
+  ret void
+; CHECK: ret void
+}




More information about the llvm-commits mailing list