[Inlining] Teach the inliner pass to inline through statepoints.

[sanjoy]

Summary

The cost analysis pass does not currently grok statepoints. We only
inline through a gc_statepoint if the call alwaysinline.

Depends on D10632

[sanjoy]

ping, so that llvm-commits sees this (forgot to subscribe llvm-commits when I created the patch).

[sanjoy]

Please ignore this change. I thought I had run the test suite after this, but I had not and a bunch of test fail with this applied. I'll put up an update and fixed change tomorrow.

[sanjoy]

• update with fixed patch!

[reames]

include/llvm/IR/CallSite.h

107

a) comments! both here on getCalledFunction to distinguish
b) naming - "meta" isn't real descriptive. Possible getCalledFunctionUnwrapped?

lib/Transforms/IPO/Inliner.cpp

457

Again, it's really not clear to me that the extra level of abstraction is worthwhile here.

613

Given we already have the getMetaCallDestination routine on callsite, I'm not sure what this extra complexity gains us.

lib/Transforms/Utils/InlineFunction.cpp

165

This looks unrelated?

[pgavlin]

include/llvm/IR/CallSite.h

107

This feels a bit ugly to me, esp. since the information about the location of the actual callee in the statepoint argument list is also present as a constant in Statepoint.h. Would it perhaps make sense to split StatepointBase into 2 types--a base type that provides argument accessors and can be included next to CallSite, and a derived type that provides getRelocates()?

Bikeshedding a bit, I'd probably also rename this to getActualCalledFunction() or similar.

lib/Transforms/IPO/Inliner.cpp

139–148

Should this simply be using getMetaCallDestination()?

457–473

This abstraction seems unnecessary: why not just s/CS.getCalledFunction()/CS.getMetaCallDestination()?

[sanjoy]

• remove InlineSite
• rename the accessor on Statepoint to getTargetFunction. I chose this name because the statepoint documentation uses the term "target"; but more bikeshedding is welcome.

[sanjoy]

include/llvm/IR/CallSite.h

107

I think there is scope to (at least) extract out a StatepointOffsets struct with all of the constants encoded as static const ints and include that in CallSite.h as well as Statepoint.h. Do you think something like that will be sufficient?

[sanjoy]

I've merged D10633 and D10632 into D10758. Their combined size is not too large IMO, and keeping them together lets me avoid adding unneeded complexity via flags like -spp-dont-pre-inline-polls just for testing.

include/llvm/IR/CallSite.h

 #ifndef LLVM_IR_CALLSITE_H
 #define LLVM_IR_CALLSITE_H
 
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/Intrinsics.h"
 
 namespace llvm {
 
 class CallInst;
 class InvokeInst;
 
 template <typename FunTy = const Function,
           typename ValTy = const Value,
public:
 
   /// setCalledFunction - Set the callee to the specified value.
   ///
   void setCalledFunction(Value *V) {
     assert(getInstruction() && "Not a call or invoke instruction!");
     *getCallee() = V;
   }
 
+  FunTy *getMetaCallDestination() const {
+    FunTy *LiteralDest = getCalledFunction();
+    if (LiteralDest->getIntrinsicID() == Intrinsic::experimental_gc_statepoint)
+      return dyn_cast<FunTy>(getArgument(2));
+
+    return LiteralDest;
+  }
+
   /// isCallee - Determine whether the passed iterator points to the
   /// callee operand's Use.
   bool isCallee(Value::const_user_iterator UI) const {
     return isCallee(&UI.getUse());
   }
 
   /// Determine whether this Use is the callee operand's Use.
   bool isCallee(const Use *U) const { return getCallee() == U; }

lib/Analysis/IPA/InlineCost.cpp

return attributeMatches(Caller, Callee, "target-cpu") &&
          attributeMatches(Caller, Callee, "target-features") &&
          attributeMatches(Caller, Callee, Attribute::SanitizeAddress) &&
          attributeMatches(Caller, Callee, Attribute::SanitizeMemory) &&
          attributeMatches(Caller, Callee, Attribute::SanitizeThread);
 }
 
 InlineCost InlineCostAnalysis::getInlineCost(CallSite CS, Function *Callee,
                                              int Threshold) {
-  // Cannot inline indirect calls.
-  if (!Callee)
+  // Cannot inline indirect calls or calls through intrinsics.
+  if (!Callee || Callee->isIntrinsic())
     return llvm::InlineCost::getNever();
 
   // Calls to functions with always-inline attributes should be inlined
   // whenever possible.
   if (CS.hasFnAttr(Attribute::AlwaysInline)) {
     if (isInlineViable(*Callee))
       return llvm::InlineCost::getAlways();
     return llvm::InlineCost::getNever();

lib/Transforms/IPO/InlineAlways.cpp

 /// a very simple and boring direct walk of the instructions looking for
 /// impossible-to-inline constructs.
 ///
 /// Note, it would be possible to go to some lengths to cache the information
 /// computed here, but as we only expect to do this for relatively few and
 /// small functions which have the explicit attribute to force inlining, it is
 /// likely not worth it in practice.
 InlineCost AlwaysInliner::getInlineCost(CallSite CS) {
-  Function *Callee = CS.getCalledFunction();
+  Function *Callee = CS.getMetaCallDestination();
 
   // Only inline direct calls to functions with always-inline attributes
   // that are viable for inlining. FIXME: We shouldn't even get here for
   // declarations.
   if (Callee && !Callee->isDeclaration() &&
       CS.hasFnAttr(Attribute::AlwaysInline) &&
       ICA->isInlineViable(*Callee))
     return InlineCost::getAlways();

lib/Transforms/IPO/Inliner.cpp

 /// This function also does some basic book-keeping to update the IR.  The
 /// InlinedArrayAllocas map keeps track of any allocas that are already
 /// available from other functions inlined into the caller.  If we are able to
 /// inline this call site we attempt to reuse already available allocas or add
 /// any new allocas to the set if not possible.
 static bool InlineCallIfPossible(CallSite CS, InlineFunctionInfo &IFI,
                                  InlinedArrayAllocasTy &InlinedArrayAllocas,
                                  int InlineHistory, bool InsertLifetime) {
-  Function *Callee = CS.getCalledFunction();
   Function *Caller = CS.getCaller();
 
+  Function *Callee = nullptr;
+  bool InlineResult = false;
+
   // Try to inline the function.  Get the list of static allocas that were
   // inlined.
-  if (!InlineFunction(CS, IFI, InsertLifetime))
+  if (Statepoint SP = Statepoint(CS)) {
+    Callee = cast<Function>(SP.getActualCallee());
+    InlineResult = InlineFunction(SP, IFI, InsertLifetime);
+  } else {
+    Callee = CS.getCalledFunction();
+    InlineResult = InlineFunction(CS, IFI, InsertLifetime);
+  }
+
+  if (!InlineResult)
     return false;
 
   AdjustCallerSSPLevel(Caller, Callee);
 
   // Look at all of the allocas that we inlined through this call site.  If we
   // have already inlined other allocas through other calls into this function,
   // then we know that they have disjoint lifetimes and that we can merge them.
   //
   // There are many heuristics possible for merging these allocas, and the
bool Inliner::runOnSCC(CallGraphSCC &SCC) {
   SmallPtrSet<Function*, 8> SCCFunctions;
   DEBUG(dbgs() << "Inliner visiting SCC:");
   for (CallGraphNode *Node : SCC) {
     Function *F = Node->getFunction();
     if (F) SCCFunctions.insert(F);
     DEBUG(dbgs() << " " << (F ? F->getName() : "INDIRECTNODE"));
   }
 
+  class InlineTask {
+    Function *Callee;
+    Function *Caller;
+    CallSite CS;
+    int InlineHistoryID;
+
+  public:
+    explicit InlineTask(Function *Callee, Function *Caller, CallSite CS,
+                        int InlineHistoryID = -1)
+        : Callee(Callee), Caller(Caller), CS(CS),
+          InlineHistoryID(InlineHistoryID) {}
+
+    Function *getCallee() const { return Callee; }
+    Function *getCaller() const { return Caller; }
+    CallSite getCallSite() const { return CS; }
+    int getInlineHistoryID() const { return InlineHistoryID; }
+  };
+
   // Scan through and identify all call sites ahead of time so that we only
   // inline call sites in the original functions, not call sites that result
   // from inlining other functions.
-  SmallVector<std::pair<CallSite, int>, 16> CallSites;
-  
+  SmallVector<InlineTask, 16> InlineTaskList;
   // When inlining a callee produces new call sites, we want to keep track of
   // the fact that they were inlined from the callee.  This allows us to avoid
   // infinite inlining in some obscure cases.  To represent this, we use an
   // index into the InlineHistory vector.
   SmallVector<std::pair<Function*, int>, 8> InlineHistory;
 
   for (CallGraphNode *Node : SCC) {
     Function *F = Node->getFunction();
     if (!F) continue;
     
     for (BasicBlock &BB : *F)
       for (Instruction &I : BB) {
         CallSite CS(cast<Value>(&I));
-        // If this isn't a call, or it is a call to an intrinsic, it can
-        // never be inlined.
-        if (!CS || isa<IntrinsicInst>(I))
+        // If this isn't a call never be inlined.
+        if (!CS)
           continue;
-        
+        // We can inline through gc_statepoint calls.  Calls to other intrinsics
+        // cannot be inlined.
+        if (isa<IntrinsicInst>(I) && !isStatepoint(I))
+          continue;
+
+        Function *CalledFunction = CS.getMetaCallDestination();
+
         // If this is a direct call to an external function, we can never inline
         // it.  If it is an indirect call, inlining may resolve it to be a
         // direct call, so we keep it.
-        if (CS.getCalledFunction() && CS.getCalledFunction()->isDeclaration())
+        if (CalledFunction && CalledFunction->isDeclaration())
           continue;
-        
-        CallSites.push_back(std::make_pair(CS, -1));
+        InlineTaskList.emplace_back(CS.getCaller(), CalledFunction, CS);
       }
   }
 
-  DEBUG(dbgs() << ": " << CallSites.size() << " call sites.\n");
+  DEBUG(dbgs() << ": " << InlineTaskList.size() << " call sites.\n");
 
   // If there are no calls in this function, exit early.
-  if (CallSites.empty())
+  if (InlineTaskList.empty())
     return false;
   
   // Now that we have all of the call sites, move the ones to functions in the
   // current SCC to the end of the list.
-  unsigned FirstCallInSCC = CallSites.size();
+  unsigned FirstCallInSCC = InlineTaskList.size();
   for (unsigned i = 0; i < FirstCallInSCC; ++i)
-    if (Function *F = CallSites[i].first.getCalledFunction())
+    if (Function *F = InlineTaskList[i].getCallee())
       if (SCCFunctions.count(F))
-        std::swap(CallSites[i--], CallSites[--FirstCallInSCC]);
-
-  
+        std::swap(InlineTaskList[i--], InlineTaskList[--FirstCallInSCC]);
   InlinedArrayAllocasTy InlinedArrayAllocas;
   InlineFunctionInfo InlineInfo(&CG, AA, ACT);
 
   // Now that we have all of the call sites, loop over them and inline them if
   // it looks profitable to do so.
   bool Changed = false;
   bool LocalChange;
   do {
     LocalChange = false;
     // Iterate over the outer loop because inlining functions can cause indirect
     // calls to become direct calls.
-    // CallSites may be modified inside so ranged for loop can not be used.
-    for (unsigned CSi = 0; CSi != CallSites.size(); ++CSi) {
-      CallSite CS = CallSites[CSi].first;
-      
-      Function *Caller = CS.getCaller();
-      Function *Callee = CS.getCalledFunction();
+    // InlineTaskList may be modified inside so ranged for loop can not be used.
+    for (unsigned CSi = 0; CSi != InlineTaskList.size(); ++CSi) {
+      CallSite CS = InlineTaskList[CSi].getCallSite();
+      Function *Caller = InlineTaskList[CSi].getCaller();
+      Function *Callee = InlineTaskList[CSi].getCallee();
 
       // If this call site is dead and it is to a readonly function, we should
       // just delete the call instead of trying to inline it, regardless of
       // size.  This happens because IPSCCP propagates the result out of the
       // call and then we're left with the dead call.
       if (isInstructionTriviallyDead(CS.getInstruction(), TLI)) {
         DEBUG(dbgs() << "    -> Deleting dead call: "
                      << *CS.getInstruction() << "\n");
         // Update the call graph by deleting the edge from Callee to Caller.
         CG[Caller]->removeCallEdgeFor(CS);
         CS.getInstruction()->eraseFromParent();
         ++NumCallsDeleted;
       } else {
         // We can only inline direct calls to non-declarations.
         if (!Callee || Callee->isDeclaration()) continue;
       
         // If this call site was obtained by inlining another function, verify
         // that the include path for the function did not include the callee
         // itself.  If so, we'd be recursively inlining the same function,
         // which would provide the same callsites, which would cause us to
         // infinitely inline.
-        int InlineHistoryID = CallSites[CSi].second;
+        int InlineHistoryID = InlineTaskList[CSi].getInlineHistoryID();
         if (InlineHistoryID != -1 &&
             InlineHistoryIncludes(Callee, InlineHistoryID, InlineHistory))
           continue;
         
         LLVMContext &CallerCtx = Caller->getContext();
 
         // Get DebugLoc to report. CS will be invalid after Inliner.
         DebugLoc DLoc = CS.getInstruction()->getDebugLoc();
for (unsigned CSi = 0; CSi != InlineTaskList.size(); ++CSi) {
         // onto our worklist to process.  They are useful inline candidates.
         if (!InlineInfo.InlinedCalls.empty()) {
           // Create a new inline history entry for this, so that we remember
           // that these new callsites came about due to inlining Callee.
           int NewHistoryID = InlineHistory.size();
           InlineHistory.push_back(std::make_pair(Callee, InlineHistoryID));
 
           for (Value *Ptr : InlineInfo.InlinedCalls)
-            CallSites.push_back(std::make_pair(CallSite(Ptr), NewHistoryID));
+            InlineTaskList.emplace_back(Callee, Caller, CallSite(Ptr),
+                                        NewHistoryID);
         }
       }
       
       // If we inlined or deleted the last possible call site to the function,
       // delete the function body now.
       if (Callee && Callee->use_empty() && Callee->hasLocalLinkage() &&
           // TODO: Can remove if in SCC now.
           !SCCFunctions.count(Callee) &&
           
for (unsigned CSi = 0; CSi != InlineTaskList.size(); ++CSi) {
         ++NumDeleted;
       }
 
       // Remove this call site from the list.  If possible, use 
       // swap/pop_back for efficiency, but do not use it if doing so would
       // move a call site to a function in this SCC before the
       // 'FirstCallInSCC' barrier.
       if (SCC.isSingular()) {
-        CallSites[CSi] = CallSites.back();
-        CallSites.pop_back();
+        InlineTaskList[CSi] = InlineTaskList.back();
+        InlineTaskList.pop_back();
       } else {
-        CallSites.erase(CallSites.begin()+CSi);
+        InlineTaskList.erase(InlineTaskList.begin() + CSi);
       }
       --CSi;
 
       Changed = true;
       LocalChange = true;
     }
   } while (LocalChange);
 

lib/Transforms/Utils/InlineFunction.cpp

 
 bool llvm::InlineFunction(Statepoint SP, InlineFunctionInfo &IFI,
                           bool InsertLifetime) {
   if (SP.gc_args_begin() != SP.gc_args_begin() ||
       SP.vm_state_begin() != SP.vm_state_end() ||
       SP.gc_transition_args_begin() != SP.gc_transition_args_end())
     return false;
 
+  Function *Callee = cast<Function>(SP.getActualCallee());
+  if (!Callee->hasGC())
+    return false;
+
+  if (StringRef(Callee->getGC()) != SP.getCallSite().getCaller()->getGC())
+    return false;
+
   Value *ReturnValOut = nullptr;
   bool InlinedMustTailCalls = false;
   if (!InlineFunctionImpl(InlineSite(SP), IFI, InsertLifetime, ReturnValOut,
                           InlinedMustTailCalls))
     return false;
 
   // TODO: add handling for unreachable code that may violate
   // defs-dominate-uses.

test/Transforms/Inline/statepoints-basic.ll

+; RUN: opt -S -always-inline < %s | FileCheck %s
+
+declare i32 @llvm.experimental.gc.statepoint.p0f_isVoidf(i64, i32, void ()*, i32, i32, ...)
+
+@A = global i32 7
+
+declare i32* @fake_personality_function()
+
+define void @callee() gc "statepoint-example" {
+ store i32 123, i32* @A
+ ret void
+}
+
+define void @callee_nogc() {
+ store i32 123, i32* @A
+ ret void
+}
+
+define i8 addrspace(1)* @test_inline_call(i8 addrspace(1)* %obj) gc "statepoint-example" {
+; CHECK-LABEL: test_inline_call(
+entry:
+; CHECK: store i32 123, i32* @A
+  call i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @callee, i32 0, i32 0, i32 0, i32 0)  alwaysinline
+  ret i8 addrspace(1)* %obj
+}
+
+define i8 addrspace(1)* @test_inline_invoke(i8 addrspace(1)* %obj) gc "statepoint-example" personality i32* ()* @fake_personality_function {
+; CHECK-LABEL: test_inline_invoke(
+entry:
+; CHECK: store i32 123, i32* @A
+  invoke i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @callee, i32 0, i32 0, i32 0, i32 0)  alwaysinline to label %normal unwind label %exc
+
+ normal:
+  ret i8 addrspace(1)* %obj
+
+ exc:
+  %lpad = landingpad { i8*, i32 } cleanup
+  ret i8 addrspace(1)* null
+}
+
+define i8 addrspace(1)* @test_no_inline_1(i8 addrspace(1)* %obj) gc "statepoint-example" {
+; We don't inline function bodies that have a mismatching GC type.
+
+; CHECK-LABEL: test_no_inline_1(
+entry:
+; CHECK-NOT: store i32 123, i32* @A
+  call i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @callee_nogc, i32 0, i32 0, i32 0, i32 0)  alwaysinline
+  ret i8 addrspace(1)* %obj
+}
+
+define i8 addrspace(1)* @test_no_inline_2(i8 addrspace(1)* %obj) gc "statepoint-example" {
+; Inlining through a statepoint that has gc arguments is not currently supported
+
+; CHECK-LABEL: test_no_inline_2(
+entry:
+; CHECK-NOT: store i32 123, i32* @A
+  call i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @callee_nogc, i32 0, i32 0, i32 0, i32 0, i8 addrspace(1)* %obj)  alwaysinline
+  ret i8 addrspace(1)* null
+}
+
+define i8 addrspace(1)* @test_no_inline_3(i8 addrspace(1)* %obj) gc "statepoint-example" {
+; Inlining through a statepoint that has deopt arguments is not currenlty supported
+
+; CHECK-LABEL: test_no_inline_3(
+entry:
+; CHECK-NOT: store i32 123, i32* @A
+  call i32 (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @callee_nogc, i32 0, i32 0, i32 0, i32 1, i32 0)  alwaysinline
+  ret i8 addrspace(1)* %obj
+}