[PATCH] D76248: Fix a bug in the inliner that causes subsequent double inlining

[Hongtao Yu via Phabricator via llvm-commits]

hoyFB updated this revision to Diff 253176.
hoyFB edited the summary of this revision.
hoyFB added a comment.
Herald added a subscriber: eraman.

Updating D76248 <https://reviews.llvm.org/D76248>: Fix a bug in the inliner that causes subsequent double inlining


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76248/new/

https://reviews.llvm.org/D76248

Files:
  llvm/lib/Transforms/IPO/Inliner.cpp
  llvm/test/Transforms/Inline/inline_call.ll


Index: llvm/test/Transforms/Inline/inline_call.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/Inline/inline_call.ll
@@ -0,0 +1,71 @@
+; Check the optimizer doesn't crash at inlining the function top and all of its callees are inlined.
+; RUN: opt < %s -O3 -S | FileCheck %s
+
+define dso_local void (...)* @second(i8** %p) {
+entry:
+  %p.addr = alloca i8**, align 8
+  store i8** %p, i8*** %p.addr, align 8
+  %tmp = load i8**, i8*** %p.addr, align 8
+  %tmp1 = load i8*, i8** %tmp, align 8
+  %tmp2 = bitcast i8* %tmp1 to void (...)*
+  ret void (...)* %tmp2
+}
+
+define dso_local void @top()  {
+entry:
+  ; CHECK: {{.*}} = {{.*}} call {{.*}} @ext
+  ; CHECK-NOT: {{.*}} = {{.*}} call {{.*}} @third
+  ; CHECK-NOT: {{.*}} = {{.*}} call {{.*}} @second
+  ; CHECK-NOT: {{.*}} = {{.*}} call {{.*}} @wrapper
+  %q = alloca i8*, align 8
+  store i8* bitcast (void ()* @third to i8*), i8** %q, align 8
+  %tmp = call void (...)* @second(i8** %q)
+  ; The call to 'wrapper' here is to ensure that its function attributes
+  ; i.e., returning its parameter and having no side effect, will be decuded
+  ; before the next round of inlining happens to 'top' to expose the bug.
+  %call =  call void (...)* @wrapper(void (...)* %tmp) 
+  ; The indirect call here is to confuse the alias analyzer so that
+  ; an incomplete graph will be built during the first round of inlining.
+  ; This allows the current function to be processed before the actual 
+  ; callee, i.e., the function 'run', is processed. Once it's simplified to 
+  ; a direct call, it also enables an additional round of inlining with all
+  ; function attributes deduced. 
+  call void (...) %call()
+  ret void
+}
+
+define dso_local void (...)* @gen() {
+entry:
+  %call = call void (...)* (...) @ext()
+  ret void (...)* %call
+}
+
+declare dso_local void (...)* @ext(...) 
+
+define dso_local void (...)* @wrapper(void (...)* %fn) {
+entry:
+  ret void (...)* %fn
+}
+
+define dso_local void @run(void (...)* %fn) {
+entry:
+  %fn.addr = alloca void (...)*, align 8
+  %f = alloca void (...)*, align 8
+  store void (...)* %fn, void (...)** %fn.addr, align 8
+  %tmp = load void (...)*, void (...)** %fn.addr, align 8
+  %call = call void (...)* @wrapper(void (...)* %tmp)
+  store void (...)* %call, void (...)** %f, align 8
+  %tmp1 = load void (...)*, void (...)** %f, align 8
+  call void (...) %tmp1()
+  ret void
+}
+
+define dso_local void @third() {
+entry:
+  %f = alloca void (...)*, align 8
+  %call = call void (...)* @gen()
+  store void (...)* %call, void (...)** %f, align 8
+  %tmp = load void (...)*, void (...)** %f, align 8
+  call void @run(void (...)* %tmp)
+  ret void
+}
\ No newline at end of file
Index: llvm/lib/Transforms/IPO/Inliner.cpp
===================================================================
--- llvm/lib/Transforms/IPO/Inliner.cpp
+++ llvm/lib/Transforms/IPO/Inliner.cpp
@@ -712,8 +712,17 @@
           int NewHistoryID = InlineHistory.size();
           InlineHistory.push_back(std::make_pair(Callee, InlineHistoryID));
 
-          for (Value *Ptr : InlineInfo.InlinedCalls)
+          for (Value *Ptr : InlineInfo.InlinedCalls) {
+#ifndef NDEBUG
+            // Make sure no dupplicates in the inline candidates. This could
+            // happen when a callsite is simpilfied to reusing the return value
+            // of another callsite during function cloning, thus the other
+            // callsite will be reconsidered here.
+            for (auto &II : CallSites)
+              assert(II.first != CallSite(Ptr));
+#endif
             CallSites.push_back(std::make_pair(CallSite(Ptr), NewHistoryID));
+          }
         }
       }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76248.253176.patch
Type: text/x-patch
Size: 3719 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200327/bbe29564/attachment.bin>