Modify verifier checks to support musttail + preallocated

[aeubanks]

Summary

preallocated and musttail can work together, but we don't want to call
@llvm.call.preallocated.setup() to modify the stack in musttail calls.
So we shouldn't have the "preallocated" operand bundle when a
preallocated call is musttail.

Also disallow use of preallocated on calls without preallocated.

Codegen not yet implemented.

[efriedma]

Should the verifier check that the preallocated argument to the call actually refers to the corresponding "preallocated" argument in the caller?

On a loosely related note, should we require that the argument to llvm.call.preallocated.setup is non-zero?

llvm/docs/LangRef.rst

1079

"although it is"

[aeubanks]

Address code review comments

[aeubanks]

In D80581#2055758, @efriedma wrote:

Should the verifier check that the preallocated argument to the call actually refers to the corresponding "preallocated" argument in the caller?

Reid has mentioned that it should be ok, if not weird, for IR to for example store then load the pointer.

On a loosely related note, should we require that the argument to llvm.call.preallocated.setup is non-zero?

Done

[efriedma]

LGTM

Reid has mentioned that it should be ok, if not weird, for IR to for example store then load the pointer.

Okay, I guess it's fine if a mismatch is always just undefined behavior, not invalid IR.

llvm/docs/LangRef.rst

``byval`` or ``byval(<ty>)``
     The byval attribute also supports an optional type argument, which must be
     the same as the pointee type of the argument.
 
     The byval attribute also supports specifying an alignment with the
     align attribute. It indicates the alignment of the stack slot to
     form and the known alignment of the pointer specified to the call
     site. If the alignment is not specified, then the code generator
     makes a target-specific assumption.
+
+.. _attr_preallocated:
+
 ``preallocated(<ty>)``
     This indicates that the pointer parameter should really be passed by
     value to the function, and that the pointer parameter's pointee has
     already been initialized before the call instruction. This attribute
     is only valid on LLVM pointer arguments. The argument must be the value
     returned by the appropriate
-    :ref:`llvm.call.preallocated.arg<int_call_preallocated_arg>`, although is
-    ignored during codegen.
-
-    Any function call with a ``preallocated`` attribute in any parameter
-    must have a ``"preallocated"`` operand bundle.
+    :ref:`llvm.call.preallocated.arg<int_call_preallocated_arg>` on non
+    ``musttail`` calls, or the corresponding caller parameter in ``musttail``
+    calls, although it is ignored during codegen.
+
+    A non ``musttail`` function call with a ``preallocated`` attribute in
+    any parameter must have a ``"preallocated"`` operand bundle. A ``musttail``
+    function call cannot have a ``"preallocated"`` operand bundle.
 
     The preallocated attribute requires a type argument, which must be
     the same as the pointee type of the argument.
 
     The preallocated attribute also supports specifying an alignment with the
     align attribute. It indicates the alignment of the stack slot to
     form and the known alignment of the pointer specified to the call
     site. If the alignment is not specified, then the code generator

 #. The optional ``tail`` and ``musttail`` markers indicate that the optimizers
    should perform tail call optimization. The ``tail`` marker is a hint that
    `can be ignored <CodeGenerator.html#sibcallopt>`_. The ``musttail`` marker
    means that the call must be tail call optimized in order for the program to
    be correct. The ``musttail`` marker provides these guarantees:
 
    #. The call will not cause unbounded stack growth if it is part of a
       recursive cycle in the call graph.
-   #. Arguments with the :ref:`inalloca <attr_inalloca>` attribute are
-      forwarded in place.
+   #. Arguments with the :ref:`inalloca <attr_inalloca>` or
+      :ref:`preallocated <attr_preallocated>` attribute are forwarded in place.
    #. If the musttail call appears in a function with the ``"thunk"`` attribute
       and the caller and callee both have varargs, than any unprototyped
       arguments in register or memory are forwarded to the callee. Similarly,
       the return value of the callee is returned to the caller's caller, even
       if a void return type is in use.
 
    Both markers imply that the callee does not access allocas from the caller.
    The ``tail`` marker additionally implies that the callee does not access

llvm/lib/IR/Verifier.cpp

for (unsigned i = 0, e = FTy->getNumParams(); i != e; ++i) {
     if (Call.paramHasAttr(i, Attribute::ImmArg)) {
       Value *ArgVal = Call.getArgOperand(i);
       Assert(isa<ConstantInt>(ArgVal) || isa<ConstantFP>(ArgVal),
              "immarg operand has non-immediate parameter", ArgVal, Call);
     }
 
     if (Call.paramHasAttr(i, Attribute::Preallocated)) {
       Value *ArgVal = Call.getArgOperand(i);
-      Assert(Call.countOperandBundlesOfType(LLVMContext::OB_preallocated) != 0,
-             "preallocated operand requires a preallocated bundle", ArgVal,
-             Call);
+      bool hasOB =
+          Call.countOperandBundlesOfType(LLVMContext::OB_preallocated) != 0;
+      bool isMustTail = Call.isMustTailCall();
+      Assert(hasOB != isMustTail,
+             "preallocated operand either requires a preallocated bundle or "
+             "the call to be musttail (but not both)",
+             ArgVal, Call);
     }
   }
 
   if (FTy->isVarArg()) {
     // FIXME? is 'nest' even legal here?
     bool SawNest = false;
     bool SawReturned = false;
 
static AttrBuilder getParameterABIAttributes(int I, AttributeList Attrs) {
   if (Attrs.hasParamAttribute(I, Attribute::Alignment) &&
       Attrs.hasParamAttribute(I, Attribute::ByVal))
     Copy.addAlignmentAttr(Attrs.getParamAlignment(I));
   return Copy;
 }
 
 void Verifier::verifyMustTailCall(CallInst &CI) {
   Assert(!CI.isInlineAsm(), "cannot use musttail call with inline asm", &CI);
-  // FIXME: support musttail + preallocated
-  Assert(!CI.countOperandBundlesOfType(LLVMContext::OB_preallocated),
-         "musttail and preallocated not yet supported", &CI);
 
   // - The caller and callee prototypes must match.  Pointer types of
   //   parameters or return types may differ in pointee type, but not
   //   address space.
   Function *F = CI.getParent()->getParent();
   FunctionType *CallerTy = F->getFunctionType();
   FunctionType *CalleeTy = CI.getFunctionType();
   if (!CI.getCalledFunction() || !CI.getCalledFunction()->isIntrinsic()) {
for (User *U : Call.users()) {
                            "llvm.call.preallocated.setup");
         FoundCall = true;
         size_t NumPreallocatedArgs = 0;
         for (unsigned i = 0; i < UseCall->getNumArgOperands(); i++) {
           if (UseCall->paramHasAttr(i, Attribute::Preallocated)) {
             ++NumPreallocatedArgs;
           }
         }
+        Assert(NumPreallocatedArgs != 0,
+               "cannot use preallocated intrinsics on a call without "
+               "preallocated arguments");
         Assert(NumArgs->equalsInt(NumPreallocatedArgs),
                "llvm.call.preallocated.setup arg size must be equal to number "
                "of preallocated arguments "
                "at call site",
                Call, *UseCall);
         // getOperandBundle() cannot be called if more than one of the operand
         // bundle exists. There is already a check elsewhere for this, so skip
         // here if we see more than one.

llvm/test/Verifier/preallocated-invalid.ll

 define void @preallocated_attribute_type_mismatch() {
     %cs = call token @llvm.call.preallocated.setup(i32 1)
     %x = call i8* @llvm.call.preallocated.arg(token %cs, i32 0) preallocated(i32)
     %y = bitcast i8* %x to i32*
     call void @foo1(i32* preallocated(i8) %y) ["preallocated"(token %cs)]
     ret void
 }
 
-; CHECK: preallocated operand requires a preallocated bundle
+; CHECK: preallocated operand either requires a preallocated bundle or the call to be musttail
 define void @preallocated_require_bundle() {
     %cs = call token @llvm.call.preallocated.setup(i32 1)
     %x = call i8* @llvm.call.preallocated.arg(token %cs, i32 0) preallocated(i32)
     %y = bitcast i8* %x to i32*
     call void @foo1(i32* preallocated(i32) %y)
     ret void
 }
 

 
 ; CHECK: token argument must be a llvm.call.preallocated.setup
 define void @preallocated_arg_token() {
     %t = call token @llvm.what()
     %x = call i8* @llvm.call.preallocated.arg(token %t, i32 1) preallocated(i32)
     ret void
 }
 
-; CHECK: musttail and preallocated not yet supported
-define void @musttail() {
+; CHECK: cannot use preallocated intrinsics on a call without preallocated arguments
+define void @preallocated_no_preallocated_args() {
     %cs = call token @llvm.call.preallocated.setup(i32 0)
-    musttail call void @foo0() ["preallocated"(token %cs)]
+    call void @foo0() ["preallocated"(token %cs)]
+    ret void
+}
+
+; CHECK: preallocated operand either requires a preallocated bundle or the call to be musttail
+define void @musttail_and_bundle(i32* preallocated(i32) %a) {
+    %cs = call token @llvm.call.preallocated.setup(i32 0)
+    musttail call void @musttail_and_bundle(i32* preallocated(i32) %a) ["preallocated"(token %cs)]
+    ret void
+}
+
+; CHECK: cannot guarantee tail call due to mismatched ABI impacting function attributes
+define void @musttail_attr_no_match(i32* preallocated(i32) %a) {
+    musttail call void @musttail_and_bundle(i32* %a)
     ret void
 }

llvm/test/Verifier/preallocated-valid.ll

 ; RUN: opt -S %s -verify
 
 declare token @llvm.call.preallocated.setup(i32)
 declare i8* @llvm.call.preallocated.arg(token, i32)
 
 declare void @foo1(i32* preallocated(i32))
+declare i64 @foo1_i64(i32* preallocated(i32))
 declare void @foo2(i32* preallocated(i32), i32*, i32* preallocated(i32))
 
 define void @preallocated() {
     %cs = call token @llvm.call.preallocated.setup(i32 1)
     %x = call i8* @llvm.call.preallocated.arg(token %cs, i32 0) preallocated(i32)
     %y = bitcast i8* %x to i32*
     call void @foo1(i32* preallocated(i32) %y) ["preallocated"(token %cs)]
     ret void
define void @preallocated_num_args() {
     %x = call i8* @llvm.call.preallocated.arg(token %cs, i32 0) preallocated(i32)
     %x1 = bitcast i8* %x to i32*
     %y = call i8* @llvm.call.preallocated.arg(token %cs, i32 1) preallocated(i32)
     %y1 = bitcast i8* %y to i32*
     %a = inttoptr i32 0 to i32*
     call void @foo2(i32* preallocated(i32) %x1, i32* %a, i32* preallocated(i32) %y1) ["preallocated"(token %cs)]
     ret void
 }
+
+define void @preallocate_musttail(i32* preallocated(i32) %a) {
+    musttail call void @foo1(i32* preallocated(i32) %a)
+    ret void
+}
+
+define i64 @preallocate_musttail_i64(i32* preallocated(i32) %a) {
+    %r = musttail call i64 @foo1_i64(i32* preallocated(i32) %a)
+    ret i64 %r
+}