[PATCH] D109294: [Inline] Remove dead users before checking if function has one use (PR51667)

[Erik Desjardins via Phabricator via llvm-commits]

erikdesjardins created this revision.
erikdesjardins added a reviewer: nikic.
Herald added subscribers: haicheng, hiraditya, eraman.
erikdesjardins requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Otherwise, inlining costs may be pessimized by removed/replaced
instructions.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109294

Files:
  llvm/lib/Analysis/InlineCost.cpp
  llvm/test/Transforms/Inline/inline-cost-dead-users.ll
  llvm/test/Transforms/Inline/last-callsite.ll


Index: llvm/test/Transforms/Inline/last-callsite.ll
===================================================================
--- llvm/test/Transforms/Inline/last-callsite.ll
+++ llvm/test/Transforms/Inline/last-callsite.ll
@@ -240,8 +240,8 @@
   ret void
 }
 
-define void @test4() {
-; CHECK-LABEL: define void @test4()
+define void @test4(void (i1)** %sink) {
+; CHECK-LABEL: define void @test4(void (i1)** %sink)
 entry:
   ; The first call is trivial to inline due to the argument. However this
   ; argument also uses the function being called as part of a complex
@@ -260,10 +260,9 @@
   ; constant expression cannot be inlined because the constant expression forms
   ; a second use. If this part starts failing we need to use more complex
   ; constant expressions to reference a particular function with them.
-  %sink = alloca i1
-  store volatile i1 icmp ne (i64 ptrtoint (void (i1)* @test4_g to i64), i64 ptrtoint(void (i1)* @test4_g to i64)), i1* %sink
+  store void (i1)* @test4_g, void (i1)** %sink
   call void @test4_g(i1 true)
-; CHECK: store volatile i1 false
+; CHECK: store void (i1)* @test4_g, void (i1)** %sink
 ; CHECK: call void @test4_g(i1 true)
 
   ret void
Index: llvm/test/Transforms/Inline/inline-cost-dead-users.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/Inline/inline-cost-dead-users.ll
@@ -0,0 +1,30 @@
+; Test to ensure that the inlining cost model isn't tripped up by dead constant users.
+; In this case, the call to g via a bitcasted function pointer is canonicalized to
+; a direct call to g with bitcasted arguments, leaving the original bitcast
+; as a dead use of g.
+
+; RUN: opt < %s -instcombine -inline -pass-remarks=inline -S 2>&1 \
+; RUN:     | FileCheck %s
+
+; CHECK: 'f' inlined into 'h' with (cost=-15035, threshold=337)
+; CHECK: 'g' inlined into 'h' with (cost=-15035, threshold=337)
+
+%0 = type { i64, i64, i64 }
+%1 = type { i64, i64, i64 }
+
+define internal void @f(%0* align 8 %a) unnamed_addr {
+start:
+  ret void
+}
+
+define internal void @g(%0* align 8 %a) unnamed_addr {
+start:
+  ret void
+}
+
+define void @h(%0* align 8 %a, %1* align 8 %b) unnamed_addr {
+start:
+  call void @f(%0* align 8 %a)
+  call void bitcast (void (%0*)* @g to void (%1*)*)(%1* align 8 %b)
+  ret void
+}
Index: llvm/lib/Analysis/InlineCost.cpp
===================================================================
--- llvm/lib/Analysis/InlineCost.cpp
+++ llvm/lib/Analysis/InlineCost.cpp
@@ -1859,6 +1859,7 @@
   SingleBBBonus = Threshold * SingleBBBonusPercent / 100;
   VectorBonus = Threshold * VectorBonusPercent / 100;
 
+  F.removeDeadConstantUsers();
   bool OnlyOneCallAndLocalLinkage =
       F.hasLocalLinkage() && F.hasOneUse() && &F == Call.getCalledFunction();
   // If there is only one call of the function, and it has internal linkage,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D109294.370814.patch
Type: text/x-patch
Size: 2858 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210905/10ec773f/attachment.bin>