[PartialInliner] Do not do partial inlining for functions with non-call references

[davidxl]

Summary

This patch fixes another bug found when enabling partial inlining. The partial inliner does not check if the use of the function is a call use or not and blindly replace uses and forces inlining (of the wrong target). It can cause crashes during compile (e.g. with the test case)

[davide]

Yes, I was reducing the same. Some comments, but I had the same fix.
Are you running with -O3 or LTO? Where are you putting the inliner?

lib/Transforms/IPO/PartialInlining.cpp

89

SmallVector<> maybe? (up to you)

96–97

Is this bit needed?
I assume the invariant F != nullptr should always hold, at which point if we don't set Callee, so Callee != F is always true and we return false.
Does it make sense to you?

[efriedma]

llvm/trunk/lib/Transforms/IPO/PartialInlining.cpp

104 ↗

(On Diff #96236)

Can you just call Function::hasAddressTaken instead?

lib/Transforms/IPO/PartialInlining.cpp

if (isa<ReturnInst>(BB->getTerminator())) {
       ReturnCount++;
     } else
       NonReturnBlock = BB;
   }
 
   if (ReturnCount != 1)
     return nullptr;
 
+  auto canAllUsesBeReplaced = [](Function *F) {
+    std::vector<User *> Users(F->user_begin(), F->user_end());
+    for (User *User : Users) {
+      Function *Callee = nullptr;
+      if (CallInst *CI = dyn_cast<CallInst>(User))
+        Callee = CallSite(CI).getCalledFunction();
+      else if (InvokeInst *II = dyn_cast<InvokeInst>(User))
+        Callee = CallSite(II).getCalledFunction();
+      else
+        return false;
+      if (Callee != F)
+        return false;
+    }
+
+    return true;
+  };
+
+  if (!canAllUsesBeReplaced(F))
+    return nullptr;
+
   // Clone the function, so that we can hack away on it.
   ValueToValueMapTy VMap;
   Function *DuplicateFunction = CloneFunction(F, VMap);
   DuplicateFunction->setLinkage(GlobalValue::InternalLinkage);
   BasicBlock *NewEntryBlock = cast<BasicBlock>(VMap[EntryBlock]);
   BasicBlock *NewReturnBlock = cast<BasicBlock>(VMap[ReturnBlock]);
   BasicBlock *NewNonReturnBlock = cast<BasicBlock>(VMap[NonReturnBlock]);
 

test/Transforms/CodeExtractor/PartialInlineCallRef.ll

+; RUN: opt < %s -partial-inliner -S  | FileCheck %s
+; RUN: opt < %s -passes=partial-inliner -S  | FileCheck %s
+
+
+; Function Attrs: nounwind
+declare void @foo(...) local_unnamed_addr #0
+
+; Function Attrs: noinline
+define i32 @caller(i32 (i32)* nocapture %arg, i32 (i32)* nocapture %arg1, i32 %arg2) local_unnamed_addr #1 {
+bb:
+  %tmp = tail call i32 %arg(i32 %arg2) #0
+  %tmp3 = tail call i32 %arg1(i32 %arg2) #0
+  %tmp4 = add nsw i32 %tmp3, %tmp
+  ret i32 %tmp4
+}
+
+; Function Attrs: nounwind
+define i32 @bar(i32 %arg) #0 {
+bb:
+  %tmp = icmp slt i32 %arg, 0
+  br i1 %tmp, label %bb1, label %bb2
+
+bb1:                                              ; preds = %bb
+  tail call void (...) @foo() #0
+  tail call void (...) @foo() #0
+  tail call void (...) @foo() #0
+  tail call void (...) @foo() #0
+  tail call void (...) @foo() #0
+  tail call void (...) @foo() #0
+  tail call void (...) @foo() #0
+  tail call void (...) @foo() #0
+  tail call void (...) @foo() #0
+  br label %bb2
+
+bb2:                                              ; preds = %bb1, %bb
+  %tmp3 = phi i32 [ 0, %bb1 ], [ 1, %bb ]
+  ret i32 %tmp3
+}
+
+; Function Attrs: nounwind
+define i32 @dummy_caller(i32 %arg) local_unnamed_addr #0 {
+bb:
+; CHECK-LABEL: @dummy_caller
+; check that caller is not wrongly inlined by partial inliner
+; CHECK: call i32 @caller
+; CHECK-NOT: call .* @bar
+  %tmp = tail call i32 @caller(i32 (i32)* nonnull @bar, i32 (i32)* nonnull @bar, i32 %arg)
+  ret i32 %tmp
+}
+
+attributes #0 = { nounwind }
+attributes #1 = { noinline }
+
+!llvm.ident = !{!0}
+
+!0 = !{!"clang version 5.0.0 (trunk 300897) (llvm/trunk 300947)"}