ArgumentPromotion: Don't hack on functions containing musttail calls

[rnk]

Summary

The prototypes need to match exactly in order to preserve ABI
guarantees. Hypothetically, if we can promote both the callee and the
caller all together, we can do argument promotion, but that is a
separable issue.

[rnk]

• Add an assert near the tail call removal that I found

[nicholas]

Isn't this overly conservative?

define internal void @aaa() {
  %X = ...
  %Y = ...
  call void @bbb(i8 %X, i8 %Y)
  ret void
}

define internal void @bbb(i8 %X, i8 %Y) {
  musttail call void @ccc(i8 %X, i8 %Y)
  ret void
}

define internal void @ccc(i8 %X, i8 %Y) {
  call @use(i8 %X)
  ret void
}

You can totally knock out %Y, right?

[rnk]

In D4352#6, @nicholas wrote:

Isn't this overly conservative?

define internal void @aaa() {
  %X = ...
  %Y = ...
  call void @bbb(i8 %X, i8 %Y)
  ret void
}

define internal void @bbb(i8 %X, i8 %Y) {
  musttail call void @ccc(i8 %X, i8 %Y)
  ret void
}

define internal void @ccc(i8 %X, i8 %Y) {
  call @use(i8 %X)
  ret void
}

You can totally knock out %Y, right?

That would be deadargelim, not argument promotion, but that's besides the point.

Yes, if we could teach argument promotion to promote the entire call stack of musttail callers and callees at once, then we could do that transformation.

It might be worth it if we expect functional languages to use this construct widely. For C++, musttail is used with thunks that will either be a) virtual, and therefore not promotable, or b) devirtualized, and more profitable to inline at the call site.

[rnk]

ping

[nicholas]

test/Transforms/ArgumentPromotion/tail.ll

22

This comment was the reason for my question about it being overly conservative. Could you reword this to be more specific about what we can't do?

[rnk]

• rebase
• enhance comment

[Eugene.Zelenko]

Looks like patch was not committed.

lib/Transforms/IPO/ArgumentPromotion.cpp

CallGraphNode *ArgPromotion::PromoteArguments(CallGraphNode *CGN) {
   for (Use &U : F->uses()) {
     CallSite CS(U.getUser());
     // Must be a direct call.
     if (CS.getInstruction() == nullptr || !CS.isCallee(&U)) return nullptr;
     
     if (CS.getInstruction()->getParent()->getParent() == F)
       isSelfRecursive = true;
   }
-  
+  // Third check: See if this function contains any musttail call sites.  If so,
+  // we can't hack on the calling convention without breaking musttail ABI
+  // guarantees.
+  for (auto CR : *CGN) {
+    Value *Instr = CR.first;
+    CallInst *CS = dyn_cast_or_null<CallInst>(Instr);
+    if (!CS)
+      continue;
+    if (CS->isMustTailCall())
+      return nullptr;
+  }
+
   // Check to see which arguments are promotable.  If an argument is promotable,
   // add it to ArgsToPromote.
   SmallPtrSet<Argument*, 8> ArgsToPromote;
   SmallPtrSet<Argument*, 8> ByValArgsToTransform;
   for (unsigned i = 0, e = PointerArgs.size(); i != e; ++i) {
     Argument *PtrArg = PointerArgs[i];
     Type *AgTy = cast<PointerType>(PtrArg->getType())->getElementType();
 
if (ByValArgsToTransform.count(I)) {
       AA.replaceWithNewValue(I, TheAlloca);
 
       // If the alloca is used in a call, we must clear the tail flag since
       // the callee now uses an alloca from the caller.
       for (User *U : TheAlloca->users()) {
         CallInst *Call = dyn_cast<CallInst>(U);
         if (!Call)
           continue;
+        assert(!Call->isMustTailCall());
         Call->setTailCall(false);
       }
       continue;
     }
 
     if (I->use_empty()) {
       AA.deleteValue(I);
       continue;

test/Transforms/ArgumentPromotion/tail.ll

; CHECK: call i8* @foo(%pair* %Data)
   tail call i8* @foo(%pair* %Data)
   ret void
 }
 
 define void @zed(%pair* byval %Data) {
   call void @bar(%pair* byval %Data)
   ret void
 }
+
+declare i8* @musttail_inner(%pair* byval)
+
+; Don't promote arguments of internal functions if they contain musttail calls
+; to functions that can't be promoted at the same time.
+define internal i8* @musttail_middle(%pair* byval %Data) {
+; CHECK: define internal i8* @musttail_middle(%pair* byval %Data)
+; CHECK: musttail call i8* @musttail_inner(%pair* byval %Data)
+  %r = musttail call i8* @musttail_inner(%pair* byval %Data)
+  ret i8* %r
+}
+
+define i8* @musttail_outer(%pair* byval %Data) {
+  %r = call i8* @musttail_middle(%pair* byval %Data)
+  ret i8* %r
+}