Add a new intrinsic and attribute to implement `__builtin_va_arg_pack{,_len}` in Clang

[erik.pilkington]

Summary

This patch adds an attribute inline_va_arg_pack which applies to calls/invokes and is the result of lowering __builtin_va_arg_pack(), as well as the intrinsic @llvm.va_arg_pack_len, which is the result of lowering __builtin_va_arg_pack_len(). These can only appear in available_externally functions, and are themselves lowered during inlining. You can read more about them here: https://gcc.gnu.org/onlinedocs/gcc-4.4.2/gcc/Constructing-Calls.html, I explained the motivation for them on the clang patch.

Fixes llvm.org/PR7219 & rdar://11102669

Thanks for taking a look!
Erik

[efriedma]

Maybe a silly question, but is this actually worth implementing, at this point? The new IR additions have unusual semantics that transforms are continually going to trip over; I'm not convinced you found all the places that currently need checks. And there isn't any existing code we need to be compatible with, I think. If we were designing a frontend extension from scratch, we would probably take a different approach.

llvm/docs/LangRef.rst

1408

I'm not really a fan of using an attribute like this. Currently, there are basically two ways to refer to a varargs list: va_start, and a musttail call. This is adding a third, in a way that's very easy to miss.

Not sure what it should look like, though. Maybe instead of calling the function directly, we should call an intrinsic? I guess any approach has odd semantic implications.

10475

You're computing the difference here using the number of IR arguments? That's not going to return the correct result in general; you need the frontend to compute this.

---

I wonder if we should add an attribute to denote "weird" intrinsics that can't be treated as equivalent to a function call in general. (For example, I think you have to forbid function merging on any function that calls this intrinsic.)

[erik.pilkington]

In D57635#1382135, @efriedma wrote:

Maybe a silly question, but is this actually worth implementing, at this point? The new IR additions have unusual semantics that transforms are continually going to trip over; I'm not convinced you found all the places that currently need checks. And there isn't any existing code we need to be compatible with, I think. If we were designing a frontend extension from scratch, we would probably take a different approach.

What kind of approach are you thinking? For us, this isn't about being compatible with GCC as much as it is about getting the feature. Doing something different here would be fine as long as its "better enough" to justify deviating from GCC compatibility. Even though no existing code depends on it, compatibility with GCC would still be nice to have, so existing libraries that conditionally use the __builtin_va_arg_pack will just work with clang, whereas if we went with a different solution they would have to keep it disabled or write two copies of all their wrapper functions.

llvm/docs/LangRef.rst

1408

I tried this out first with an intrinsic, but it seemed like pretending that the va args were a value produced by a call that are passed as an argument in a different instruction was a more fragile and odd way of modelling this. Either way though, there will still be 3 ways of referencing va args, and I'm not sure that using an intrinsic instead of an attribute would make this corner case any more visible.

10475

You're computing the difference here using the number of IR arguments? That's not going to return the correct result in general; you need the frontend to compute this.

Oh right, good point. I'll update this on monday.

I wonder if we should add an attribute to denote "weird" intrinsics that can't be treated as equivalent to a function call in general. (For example, I think you have to forbid function merging on any function that calls this intrinsic.)

What other cases do you think are "weird" enough?

[efriedma]

In terms of the general approach, what problem are we really trying to solve? The ability to forward a variadic argument list on to another variadic function doesn't grant any semantic power if you control the implementation; you can always just use a va_list instead (e.g. convert printf to vprintf). In C++, you can also just use a variadic template. And if the goal is just to allow the compiler to emit fortified calls to known library functions, it would be more straightforward to add a flag to implicitly instrument code.

I guess if you specifically want to add a feature to C which allows you to write, in C, a function (not a macro) which wraps an interface you don't control that takes a variadic argument list, you can't do much better than __builtin_va_arg_pack().

llvm/docs/LangRef.rst

10475

Basically there's a general class of intrinsics which do something different if you transform a direct call into a call to a function which calls the intrinsic. (This is excluding intrinsics which expect certain arguments to be integer constants; those should also be marked somehow, but it's mostly orthogonal.)

Going through the list of target-independent intrinsics, I think the following fall into that category: llvm.va_start, llvm.va_end, llvm.stackprotector, llvm.returnaddress, llvm.addressofreturnaddress, llvm.sponentry, llvm.frameaddress, llvm.localescape, llvm.stacksave, llvm.stackrestore, llvm.get.dynamic.area.offset, llvm.lifetime.start, llvm.lifetime.end, llvm.invariant.start, llvm.invariant.end, some llvm.eh.* intrinsics, and some llvm.dbg.* intrinsics.

llvm/lib/Transforms/Utils/InlineFunction.cpp

1872

Forwarding like this doesn't actually work in general, if the fixed arguments of the caller don't match the fixed arguments of the callee. For certain calling conventions (like x86-64), the IR types change depending on the number of available registers.

Not sure what the correct solution is here.

[erik.pilkington]

In D57635#1382249, @efriedma wrote:

In terms of the general approach, what problem are we really trying to solve? The ability to forward a variadic argument list on to another variadic function doesn't grant any semantic power if you control the implementation; you can always just use a va_list instead (e.g. convert printf to vprintf). In C++, you can also just use a variadic template. And if the goal is just to allow the compiler to emit fortified calls to known library functions, it would be more straightforward to add a flag to implicitly instrument code.

I guess if you specifically want to add a feature to C which allows you to write, in C, a function (not a macro) which wraps an interface you don't control that takes a variadic argument list, you can't do much better than __builtin_va_arg_pack().

Yeah, this is just about getting C++-compatible fortified wrappers for libc functions. I thought that delegating to a va_list variant was no good since it looks like we don't have the ability to inline a function that calls va_start, so any fortified stuff in the body would just be discarded. I think I'm convinced that handling this in the frontend would be a simpler way of solving this though, maybe with an attribute or something. I'm going to tentatively mark this as abandoned, I'll revive it if the frontend stuff doesn't pan out.

Thanks for your help!

llvm/docs/LangRef.rst

     either not accessible by the module being compiled, or is pointed to
     by its pointer arguments. This is a weaker form of  ``argmemonly``. If the
     function reads or writes other memory, the behavior is undefined.
 ``inlinehint``
     This attribute indicates that the source code contained a hint that
     inlining this function is desirable (such as the "inline" keyword in
     C/C++). It is just a hint; it imposes no requirements on the
     inliner.
+``inline_va_arg_pack``
+    This attribute can appear on a call or invoke in a function with
+    available_externally linkage. If the function is inlined, the variadic
+    arguments of the caller will be forwarded into the call instruction. This
+    attribute is used to implement `__builtin_va_arg_pack`.
 ``jumptable``
     This attribute indicates that the function should be added to a
     jump-instruction table at code-generation time, and that all address-taken
     references to this function should be replaced with a reference to the
     appropriate jump-instruction-table function pointer. Note that this creates
     a new pointer for the original function, which means that code that depends
     on function-pointer identity can break. So, any function annotated with
     ``jumptable`` must also be ``unnamed_addr``.

 """"""""""
 
 The '``llvm.va_copy``' intrinsic works just like the ``va_copy`` macro
 available in C. In a target-dependent way, it copies the source
 ``va_list`` element into the destination ``va_list`` element. This
 intrinsic is necessary because the `` llvm.va_start`` intrinsic may be
 arbitrarily complex and require, for example, memory allocation.
 
+'``llvm.va_arg_pack_len``' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Syntax:
+"""""""
+
+::
+
+      declare i32 @llvm.va_arg_pack_len()
+
+Overview:
+"""""""""
+
+The '``llvm.va_arg_pack_len``' intrinsic folds to the number of variadic
+arguments to the current function if its enclosing function is inlined.
+
+Semantics:
+""""""""""
+
+This intrinsic can only appear in an ``available_externally`` variadic function.
+As the enclosing function is being inlined, the inliner folds the call to the
+difference of the total number of arguments at the call site and the number of
+declared parameters.
+
 Accurate Garbage Collection Intrinsics
 --------------------------------------
 
 LLVM's support for `Accurate Garbage Collection <GarbageCollection.html>`_
 (GC) requires the frontend to generate code containing appropriate intrinsic
 calls and select an appropriate GC strategy which knows how to lower these
 intrinsics in a manner which is appropriate for the target collector.
 

llvm/include/llvm/Bitcode/LLVMBitCodes.h

   ATTR_KIND_WRITEONLY = 52,
   ATTR_KIND_SPECULATABLE = 53,
   ATTR_KIND_STRICT_FP = 54,
   ATTR_KIND_SANITIZE_HWADDRESS = 55,
   ATTR_KIND_NOCF_CHECK = 56,
   ATTR_KIND_OPT_FOR_FUZZING = 57,
   ATTR_KIND_SHADOWCALLSTACK = 58,
   ATTR_KIND_SPECULATIVE_LOAD_HARDENING = 59,
+  ATTR_KIND_INLINE_VA_ARG_PACK = 60,
 };
 
 enum ComdatSelectionKindCodes {
   COMDAT_SELECTION_KIND_ANY = 1,
   COMDAT_SELECTION_KIND_EXACT_MATCH = 2,
   COMDAT_SELECTION_KIND_LARGEST = 3,
   COMDAT_SELECTION_KIND_NO_DUPLICATES = 4,
   COMDAT_SELECTION_KIND_SAME_SIZE = 5,

llvm/include/llvm/IR/Attributes.td

 /// Target-independent string attributes.
 def LessPreciseFPMAD : StrBoolAttr<"less-precise-fpmad">;
 def NoInfsFPMath : StrBoolAttr<"no-infs-fp-math">;
 def NoNansFPMath : StrBoolAttr<"no-nans-fp-math">;
 def UnsafeFPMath : StrBoolAttr<"unsafe-fp-math">;
 def NoJumpTables : StrBoolAttr<"no-jump-tables">;
 def ProfileSampleAccurate : StrBoolAttr<"profile-sample-accurate">;
 
+/// This applies to a CallInst to a function whose last argument in C was a call
+/// to builtin_va_arg_pack(). When inlining, the vaargs call arguments are
+/// appended to the call arguments of this function, and the attribute is
+/// stripped.
+def InlineVAArgPack : EnumAttr<"inline_va_arg_pack">;
+
 class CompatRule<string F> {
   // The name of the function called to check the attribute of the caller and
   // callee and decide whether inlining should be allowed. The function's
   // signature must match "bool(const Function&, const Function &)", where the
   // first parameter is the reference to the caller and the second parameter is
   // the reference to the callee. It must return false if the attributes of the
   // caller and callee are incompatible, and true otherwise.
   string CompatFunc = F;

llvm/include/llvm/IR/InstrTypes.h

   bool isConvergent() const { return hasFnAttr(Attribute::Convergent); }
   void setConvergent() {
     addAttribute(AttributeList::FunctionIndex, Attribute::Convergent);
   }
   void setNotConvergent() {
     removeAttribute(AttributeList::FunctionIndex, Attribute::Convergent);
   }
 
+  /// Determine if we should forward variadic arguments into this call when
+  /// inlining.
+  bool isInlineVAArgPackCall() const {
+    return Attrs.hasAttribute(AttributeList::FunctionIndex,
+                              Attribute::InlineVAArgPack);
+  }
+  void setIsInlineVAArgPackCall(bool Val) {
+    if (Val)
+      addAttribute(AttributeList::FunctionIndex, Attribute::InlineVAArgPack);
+    else
+      removeAttribute(AttributeList::FunctionIndex, Attribute::InlineVAArgPack);
+  }
+
   /// Determine if the call returns a structure through first
   /// pointer argument.
   bool hasStructRetAttr() const {
     if (getNumArgOperands() == 0)
       return false;
 
     // Be friendly and also check the callee.
     return paramHasAttr(0, Attribute::StructRet);

llvm/include/llvm/IR/Intrinsics.td

 //===--------------- Variable Argument Handling Intrinsics ----------------===//
 //
 
 def int_vastart : Intrinsic<[], [llvm_ptr_ty], [], "llvm.va_start">;
 def int_vacopy  : Intrinsic<[], [llvm_ptr_ty, llvm_ptr_ty], [],
                             "llvm.va_copy">;
 def int_vaend   : Intrinsic<[], [llvm_ptr_ty], [], "llvm.va_end">;
 
+def int_va_arg_pack_len : Intrinsic<[llvm_i32_ty], [], [],
+                                    "llvm.va_arg_pack_len">;
+
 //===------------------- Garbage Collection Intrinsics --------------------===//
 //
 def int_gcroot  : Intrinsic<[],
                             [llvm_ptrptr_ty, llvm_ptr_ty]>;
 def int_gcread  : Intrinsic<[llvm_ptr_ty],
                             [llvm_ptr_ty, llvm_ptrptr_ty],
                             [IntrReadMem, IntrArgMemOnly]>;
 def int_gcwrite : Intrinsic<[],

llvm/lib/AsmParser/LLLexer.cpp

   KEYWORD(inalloca);
   KEYWORD(cold);
   KEYWORD(convergent);
   KEYWORD(dereferenceable);
   KEYWORD(dereferenceable_or_null);
   KEYWORD(inaccessiblememonly);
   KEYWORD(inaccessiblemem_or_argmemonly);
   KEYWORD(inlinehint);
+  KEYWORD(inline_va_arg_pack);
   KEYWORD(inreg);
   KEYWORD(jumptable);
   KEYWORD(minsize);
   KEYWORD(naked);
   KEYWORD(nest);
   KEYWORD(noalias);
   KEYWORD(nobuiltin);
   KEYWORD(nocapture);

llvm/lib/AsmParser/LLParser.cpp

     case lltok::kw_builtin: B.addAttribute(Attribute::Builtin); break;
     case lltok::kw_cold: B.addAttribute(Attribute::Cold); break;
     case lltok::kw_convergent: B.addAttribute(Attribute::Convergent); break;
     case lltok::kw_inaccessiblememonly:
       B.addAttribute(Attribute::InaccessibleMemOnly); break;
     case lltok::kw_inaccessiblemem_or_argmemonly:
       B.addAttribute(Attribute::InaccessibleMemOrArgMemOnly); break;
     case lltok::kw_inlinehint: B.addAttribute(Attribute::InlineHint); break;
+    case lltok::kw_inline_va_arg_pack:
+      B.addAttribute(Attribute::InlineVAArgPack); break;
     case lltok::kw_jumptable: B.addAttribute(Attribute::JumpTable); break;
     case lltok::kw_minsize: B.addAttribute(Attribute::MinSize); break;
     case lltok::kw_naked: B.addAttribute(Attribute::Naked); break;
     case lltok::kw_nobuiltin: B.addAttribute(Attribute::NoBuiltin); break;
     case lltok::kw_noduplicate: B.addAttribute(Attribute::NoDuplicate); break;
     case lltok::kw_noimplicitfloat:
       B.addAttribute(Attribute::NoImplicitFloat); break;
     case lltok::kw_noinline: B.addAttribute(Attribute::NoInline); break;

llvm/lib/AsmParser/LLToken.h

enum Kind {
   kw_inalloca,
   kw_cold,
   kw_convergent,
   kw_dereferenceable,
   kw_dereferenceable_or_null,
   kw_inaccessiblememonly,
   kw_inaccessiblemem_or_argmemonly,
   kw_inlinehint,
+  kw_inline_va_arg_pack,
   kw_inreg,
   kw_jumptable,
   kw_minsize,
   kw_naked,
   kw_nest,
   kw_noalias,
   kw_nobuiltin,
   kw_nocapture,

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

   case Attribute::Speculatable:    return 1ULL << 54;
   case Attribute::StrictFP:        return 1ULL << 55;
   case Attribute::SanitizeHWAddress: return 1ULL << 56;
   case Attribute::NoCfCheck:       return 1ULL << 57;
   case Attribute::OptForFuzzing:   return 1ULL << 58;
   case Attribute::ShadowCallStack: return 1ULL << 59;
   case Attribute::SpeculativeLoadHardening:
     return 1ULL << 60;
+  case Attribute::InlineVAArgPack: return 1ULL << 61;
   case Attribute::Dereferenceable:
     llvm_unreachable("dereferenceable attribute not supported in raw format");
     break;
   case Attribute::DereferenceableOrNull:
     llvm_unreachable("dereferenceable_or_null attribute not supported in raw "
                      "format");
     break;
   case Attribute::ArgMemOnly:

   case bitc::ATTR_KIND_CONVERGENT:
     return Attribute::Convergent;
   case bitc::ATTR_KIND_INACCESSIBLEMEM_ONLY:
     return Attribute::InaccessibleMemOnly;
   case bitc::ATTR_KIND_INACCESSIBLEMEM_OR_ARGMEMONLY:
     return Attribute::InaccessibleMemOrArgMemOnly;
   case bitc::ATTR_KIND_INLINE_HINT:
     return Attribute::InlineHint;
+  case bitc::ATTR_KIND_INLINE_VA_ARG_PACK:
+    return Attribute::InlineVAArgPack;
   case bitc::ATTR_KIND_IN_REG:
     return Attribute::InReg;
   case bitc::ATTR_KIND_JUMP_TABLE:
     return Attribute::JumpTable;
   case bitc::ATTR_KIND_MIN_SIZE:
     return Attribute::MinSize;
   case bitc::ATTR_KIND_NAKED:
     return Attribute::Naked;

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

   case Attribute::Cold:
     return bitc::ATTR_KIND_COLD;
   case Attribute::InaccessibleMemOnly:
     return bitc::ATTR_KIND_INACCESSIBLEMEM_ONLY;
   case Attribute::InaccessibleMemOrArgMemOnly:
     return bitc::ATTR_KIND_INACCESSIBLEMEM_OR_ARGMEMONLY;
   case Attribute::InlineHint:
     return bitc::ATTR_KIND_INLINE_HINT;
+  case Attribute::InlineVAArgPack:
+    return bitc::ATTR_KIND_INLINE_VA_ARG_PACK;
   case Attribute::InReg:
     return bitc::ATTR_KIND_IN_REG;
   case Attribute::JumpTable:
     return bitc::ATTR_KIND_JUMP_TABLE;
   case Attribute::MinSize:
     return bitc::ATTR_KIND_MIN_SIZE;
   case Attribute::Naked:
     return bitc::ATTR_KIND_NAKED;

llvm/lib/IR/Attributes.cpp

std::string Attribute::getAsString(bool InAttrGrp) const {
   if (hasAttribute(Attribute::InaccessibleMemOnly))
     return "inaccessiblememonly";
   if (hasAttribute(Attribute::InaccessibleMemOrArgMemOnly))
     return "inaccessiblemem_or_argmemonly";
   if (hasAttribute(Attribute::InAlloca))
     return "inalloca";
   if (hasAttribute(Attribute::InlineHint))
     return "inlinehint";
+  if (hasAttribute(Attribute::InlineVAArgPack))
+    return "inline_va_arg_pack";
   if (hasAttribute(Attribute::InReg))
     return "inreg";
   if (hasAttribute(Attribute::JumpTable))
     return "jumptable";
   if (hasAttribute(Attribute::MinSize))
     return "minsize";
   if (hasAttribute(Attribute::Naked))
     return "naked";

llvm/lib/IR/Verifier.cpp

   case Attribute::OptForFuzzing:
   case Attribute::OptimizeNone:
   case Attribute::JumpTable:
   case Attribute::Convergent:
   case Attribute::ArgMemOnly:
   case Attribute::NoRecurse:
   case Attribute::InaccessibleMemOnly:
   case Attribute::InaccessibleMemOrArgMemOnly:
+  case Attribute::InlineVAArgPack:
   case Attribute::AllocSize:
   case Attribute::SpeculativeLoadHardening:
   case Attribute::Speculatable:
   case Attribute::StrictFP:
     return true;
   default:
     break;
   }
   return false;
 }
 
 /// Return true if this is a function attribute that can also appear on
 /// arguments.
 static bool isFuncOrArgAttr(Attribute::AttrKind Kind) {
   return Kind == Attribute::ReadOnly || Kind == Attribute::WriteOnly ||
          Kind == Attribute::ReadNone;
 }
 
+static bool isCallOnlyAttr(Attribute::AttrKind Kind) {
+  return Kind == Attribute::InlineVAArgPack;
+}
+
 void Verifier::verifyAttributeTypes(AttributeSet Attrs, bool IsFunction,
                                     const Value *V) {
   for (Attribute A : Attrs) {
     if (A.isStringAttribute())
       continue;
 
     if (isFuncOnlyAttr(A.getKindAsEnum())) {
       if (!IsFunction) {
         CheckFailed("Attribute '" + A.getAsString() +
                         "' only applies to functions!",
                     V);
         return;
       }
     } else if (IsFunction && !isFuncOrArgAttr(A.getKindAsEnum())) {
       CheckFailed("Attribute '" + A.getAsString() +
                       "' does not apply to functions!",
                   V);
       return;
+    } else if (isCallOnlyAttr(A.getKindAsEnum()) &&
+               (!isa<CallBase>(V) || !IsFunction)) {
+      CheckFailed("Attribute '" + A.getAsString() +
+                  "' only applies to call instructions");
+      return;
     }
   }
 }
 
 // VerifyParameterAttrs - Check the given attributes for an argument or return
 // value of the specified type.  The value V is printed in error messages.
 void Verifier::verifyParameterAttrs(AttributeSet Attrs, Type *Ty,
                                     const Value *V) {

          "musttail call result must be returned", Ret);
 }
 
 void Verifier::visitCallInst(CallInst &CI) {
   visitCallBase(CI);
 
   if (CI.isMustTailCall())
     verifyMustTailCall(CI);
+
+  if (CI.isInlineVAArgPackCall()) {
+    Function *FD = CI.getParent()->getParent();
+    Assert(
+        FD->hasAvailableExternallyLinkage(),
+        "inline_va_arg_pack must be used in an available_externally function");
+    Assert(FD->isVarArg(),
+           "inline_va_arg_pack can only appear in a variadic function");
+  }
 }
 
 void Verifier::visitInvokeInst(InvokeInst &II) {
   visitCallBase(II);
 
   // Verify that the first non-PHI instruction of the unwind destination is an
   // exception handling instruction.
   Assert(

     auto *Op3 = dyn_cast<ConstantInt>(Call.getArgOperand(2));
     Assert(Op3, "third argument of smul_fix must be a constant integer");
     Assert(Op3->getType()->getBitWidth() <= 32,
            "third argument of smul_fix must fit within 32 bits");
     Assert(Op3->getZExtValue() < Op1->getType()->getScalarSizeInBits(),
            "the scale of smul_fix must be less than the width of the operands");
     break;
   }
+  case Intrinsic::va_arg_pack_len: {
+    Function *FD = Call.getParent()->getParent();
+    Assert(FD->hasAvailableExternallyLinkage(),
+           "call to @llvm.va_arg_pack_len can only appear in a function with"
+           "available_externally linkage");
+    Assert(
+        FD->isVarArg(),
+        "call to @llvm.va_arg_pack_len can only appear in a variadic function");
+    break;
+  }
   };
 }
 
 /// Carefully grab the subprogram from a local scope.
 ///
 /// This carefully grabs the subprogram from a local scope, avoiding the
 /// built-in assertions that would typically fire.
 static DISubprogram *getSubprogram(Metadata *LocalScope) {

llvm/lib/Transforms/Utils/CodeExtractor.cpp

if (const CallInst *CI = dyn_cast<CallInst>(I)) {
           else
             return false;
         }
 
         // Currently, we miscompile outlined copies of eh_typid_for. There are
         // proposals for fixing this in llvm.org/PR39545.
         if (IID == Intrinsic::eh_typeid_for)
           return false;
+
+        // Calls to this intrinsic are tied to the current function.
+        if (IID == Intrinsic::va_arg_pack_len)
+          return false;
       }
+      // Calls with this attribute are tied to the current function.
+      if (CI->hasFnAttr(Attribute::InlineVAArgPack))
+        return false;
     }
   }
 
   return true;
 }
 
 /// Build a set of blocks to extract if the input blocks are viable.
 static SetVector<BasicBlock *>
if (Attr.isStringAttribute()) {
       case Attribute::SpeculativeLoadHardening:
       case Attribute::StackProtect:
       case Attribute::StackProtectReq:
       case Attribute::StackProtectStrong:
       case Attribute::StrictFP:
       case Attribute::UWTable:
       case Attribute::NoCfCheck:
         break;
+      case Attribute::InlineVAArgPack:
+        llvm_unreachable("InlineVAArgPack should never reach here");
       }
 
     newFunction->addFnAttr(Attr);
   }
   newFunction->getBasicBlockList().push_back(newRootNode);
 
   // Create an iterator to name all of the arguments we inserted.
   Function::arg_iterator AI = newFunction->arg_begin();

llvm/lib/Transforms/Utils/InlineFunction.cpp

   SmallVector<Value*,4> VarArgsToForward;
   SmallVector<AttributeSet, 4> VarArgsAttrs;
   for (unsigned i = CalledFunc->getFunctionType()->getNumParams();
        i < CS.getNumArgOperands(); i++) {
     VarArgsToForward.push_back(CS.getArgOperand(i));
     VarArgsAttrs.push_back(CS.getAttributes().getParamAttributes(i));
   }
 
+  auto forwardVAArgsToCall = [&](CallBase *CI) {
+    // Collect attributes for non-vararg parameters.
+    AttributeList Attrs = CI->getAttributes();
+    SmallVector<AttributeSet, 8> ArgAttrs;
+    if (!Attrs.isEmpty() || !VarArgsAttrs.empty()) {
+      for (unsigned ArgNo = 0; ArgNo < CI->getFunctionType()->getNumParams();
+           ++ArgNo)
+        ArgAttrs.push_back(Attrs.getParamAttributes(ArgNo));
+    }
+
+    // Add VarArg attributes.
+    ArgAttrs.append(VarArgsAttrs.begin(), VarArgsAttrs.end());
+    Attrs = AttributeList::get(CI->getContext(), Attrs.getFnAttributes(),
+                               Attrs.getRetAttributes(), ArgAttrs);
+    // Add VarArgs to existing parameters.
+    SmallVector<Value *, 6> Params(CI->arg_operands());
+    Params.append(VarArgsToForward.begin(), VarArgsToForward.end());
+
+    CallBase *NewCI = nullptr;
+    // Rebuild the call/invoke.
+    if (auto *Call = dyn_cast<CallInst>(CI)) {
+      NewCI = CallInst::Create(Call->getFunctionType(),
+                               Call->getCalledOperand(), Params, "", Call);
+    } else if (auto *Invoke = dyn_cast<InvokeInst>(CI)) {
+      NewCI = InvokeInst::Create(
+          Invoke->getFunctionType(), Invoke->getCalledOperand(),
+          Invoke->getNormalDest(), Invoke->getUnwindDest(), Params, "", Invoke);
+    } else {
+      llvm_unreachable("CallBase that isn't call or invoke?");
+    }
+
+    NewCI->setDebugLoc(CI->getDebugLoc());
+    NewCI->setAttributes(Attrs);
+    NewCI->setCallingConv(CI->getCallingConv());
+    return NewCI;
+  };
+
+  // Walk through the newly inserted basic blocks, replacing calls to
+  // @llvm.va_arg_pack_len with the number of variadic arguments, and expanding
+  // variadic arguments into calls annotated with 'inline_va_arg_pack'.
+  for (Function::iterator B = FirstNewBlock, E = Caller->end(); B != E; ++B) {
+    for (auto II = B->begin(); II != B->end();) {
+      Instruction *I = &*II++;
+      if (CallBase *CB = dyn_cast<CallBase>(I)) {
+        if (CB->isInlineVAArgPackCall()) {
+          auto *NewCB = forwardVAArgsToCall(CB);
+          NewCB->setIsInlineVAArgPackCall(false);
+          CB->replaceAllUsesWith(NewCB);
+          CB->eraseFromParent();
+          continue;
+        }
+
+        Function *CalledFunction = CB->getCalledFunction();
+        if (CalledFunction &&
+            CalledFunction->getIntrinsicID() == Intrinsic::va_arg_pack_len) {
+          unsigned Val = CS.getNumArgOperands() -
+                             CalledFunc->getFunctionType()->getNumParams();
+          Value *Result = ConstantInt::get(CB->getType(), Val);
+          CB->replaceAllUsesWith(Result);
+          CB->eraseFromParent();
+          continue;
+        }
+      }
+    }
+  }
+
   bool InlinedMustTailCalls = false, InlinedDeoptimizeCalls = false;
   if (InlinedFunctionInfo.ContainsCalls) {
     CallInst::TailCallKind CallSiteTailKind = CallInst::TCK_None;
     if (CallInst *CI = dyn_cast<CallInst>(TheCall))
       CallSiteTailKind = CI->getTailCallKind();
 
     // For inlining purposes, the "notail" marker is the same as no marker.
     if (CallSiteTailKind == CallInst::TCK_NoTail)
       CallSiteTailKind = CallInst::TCK_None;
 
     for (Function::iterator BB = FirstNewBlock, E = Caller->end(); BB != E;
          ++BB) {
       for (auto II = BB->begin(); II != BB->end();) {
         Instruction &I = *II++;
         CallInst *CI = dyn_cast<CallInst>(&I);
         if (!CI)
           continue;
 
         // Forward varargs from inlined call site to calls to the
         // ForwardVarArgsTo function, if requested, and to musttail calls.
         if (!VarArgsToForward.empty() &&
             ((ForwardVarArgsTo &&
               CI->getCalledFunction() == ForwardVarArgsTo) ||
              CI->isMustTailCall())) {
-          // Collect attributes for non-vararg parameters.
-          AttributeList Attrs = CI->getAttributes();
-          SmallVector<AttributeSet, 8> ArgAttrs;
-          if (!Attrs.isEmpty() || !VarArgsAttrs.empty()) {
-            for (unsigned ArgNo = 0;
-                 ArgNo < CI->getFunctionType()->getNumParams(); ++ArgNo)
-              ArgAttrs.push_back(Attrs.getParamAttributes(ArgNo));
-          }
-
-          // Add VarArg attributes.
-          ArgAttrs.append(VarArgsAttrs.begin(), VarArgsAttrs.end());
-          Attrs = AttributeList::get(CI->getContext(), Attrs.getFnAttributes(),
-                                     Attrs.getRetAttributes(), ArgAttrs);
-          // Add VarArgs to existing parameters.
-          SmallVector<Value *, 6> Params(CI->arg_operands());
-          Params.append(VarArgsToForward.begin(), VarArgsToForward.end());
-          CallInst *NewCI = CallInst::Create(
-              CI->getFunctionType(), CI->getCalledOperand(), Params, "", CI);
-          NewCI->setDebugLoc(CI->getDebugLoc());
-          NewCI->setAttributes(Attrs);
-          NewCI->setCallingConv(CI->getCallingConv());
+          CallInst *NewCI = cast<CallInst>(forwardVAArgsToCall(CI));
           CI->replaceAllUsesWith(NewCI);
           CI->eraseFromParent();
           CI = NewCI;
         }
 
         if (Function *F = CI->getCalledFunction())
           InlinedDeoptimizeCalls |=
               F->getIntrinsicID() == Intrinsic::experimental_deoptimize;

llvm/test/Transforms/Inline/inline-va-arg-pack.ll

+; RUN: opt -inline -S < %s | FileCheck %s
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.14.0"
+
+define available_externally i32 @wrapper(i32 %p, ...) {
+entry:
+  %retval = call i32 (i32, ...) @vaargs(i32 %p) inline_va_arg_pack
+  ret i32 %retval
+}
+
+define i32 @call_it() {
+entry:
+  %val = call i32 (i32, ...) @wrapper(i32 1, i32 2, i32 3, i32 4)
+  ret i32 %val
+}
+
+; CHECK-LABEL: define i32 @call_it
+; CHECK-NEXT: entry:
+; CHECK-NEXT: call i32 (i32, ...) @vaargs(i32 1, i32 2, i32 3, i32 4)
+; CHECK-NEXT: ret i32
+
+define i32 @call_it_many() {
+entry:
+  %val = call i32 (i32, ...) @wrapper(i32 1, i8* null)
+  %val1 = call i32 (i32, ...) @wrapper(i32 2, i32 42)
+  ret i32 0
+}
+
+; CHECK-LABEL: define i32 @call_it_many
+; CHECK-NEXT: entry:
+; CHECK-NEXT: call i32 (i32, ...) @vaargs(i32 1, i8* null)
+; CHECK-NEXT: call i32 (i32, ...) @vaargs(i32 2, i32 42)
+; CHECK-NEXT: ret
+
+define available_externally i32 @wrapper2(...) {
+entry:
+  %val1 = call i32 (...) @all_vaargs() inline_va_arg_pack
+  %val2 = call i32 (...) @all_vaargs() inline_va_arg_pack
+  %val3 = call i32 (...) @all_vaargs()
+  ret i32 0
+}
+
+define i32 @call_wrapper2() {
+entry:
+  %val = call i32 (...) @wrapper2(i32 1, i32 2, i32 3)
+  ret i32 %val
+}
+
+; CHECK-LABEL: define i32 @call_wrapper2
+; CHECK-NEXT: entry:
+; CHECK-NEXT: call i32 (...) @all_vaargs(i32 1, i32 2, i32 3)
+; CHECK-NEXT: call i32 (...) @all_vaargs(i32 1, i32 2, i32 3)
+; CHECK-NEXT: call i32 (...) @all_vaargs()
+; CHECK-NEXT: ret
+
+define available_externally i32 @uses_invoke(...) personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+entry:
+  %val = invoke i32 (...) @all_vaargs() inline_va_arg_pack
+         to label %normal unwind label %lpad
+normal:
+  ret i32 1
+
+lpad:
+  %l = landingpad { i8*, i32 } cleanup
+  ret i32 2
+}
+
+define i32 @call_invoke() {
+entry:
+  %val = call i32 (...) @uses_invoke(i32 1, i32 2, i32 3)
+  ret i32 %val
+}
+
+; CHECK-LABEL: define i32 @call_invoke
+; CHECK: invoke i32 (...) @all_vaargs(i32 1, i32 2, i32 3)
+
+declare i32 @vaargs(i32, ...)
+declare i32 @all_vaargs(...)
+declare i32 @__gxx_personality_v0(...)

llvm/test/Transforms/Inline/va-arg-pack-len.ll

+; RUN: opt -inline -S < %s | FileCheck %s --dump-input-on-failure
+
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.14.0"
+
+define available_externally i32 @inline_me(i32 %p, ...) {
+entry:
+  %val = call i32 @llvm.va_arg_pack_len()
+  ret i32 %val
+}
+
+define i32 @call_it() {
+entry:
+  %val = call i32 (i32, ...) @inline_me(i32 1, i32 2, i32 3, i32 4)
+  ret i32 %val
+}
+
+; CHECK-LABEL: define i32 @call_it
+; CHECK-NEXT: entry:
+; CHECK-NEXT: ret i32 3
+
+define i32 @noargs() {
+entry:
+  %val = call i32 (i32, ...) @inline_me(i32 1)
+  ret i32 %val
+}
+
+; CHECK-LABEL: define i32 @noargs
+; CHECK-NEXT: entry:
+; CHECK-NEXT: ret i32 0
+
+define available_externally i32 @inline_me2(...) {
+entry:
+  %val = call i32 @llvm.va_arg_pack_len()
+  ret i32 %val
+}
+
+define i32 @call_it2() {
+entry:
+  %val = call i32 (...) @inline_me2(i32 1, i32 2, i32 3, i32 4)
+  ret i32 %val
+}
+
+; CHECK-LABEL: define i32 @call_it2
+; CHECK-NEXT: entry:
+; CHECK-NEXT: ret i32 4
+
+define i32 @noargs2() {
+entry:
+  %val = call i32 (...) @inline_me2()
+  ret i32 %val
+}
+
+; CHECK-LABEL: define i32 @noargs
+; CHECK-NEXT: entry:
+; CHECK-NEXT: ret i32 0
+
+declare i32 @llvm.va_arg_pack_len()

llvm/test/Verifier/va-arg-pack.ll

+; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s --dump-input-on-failure
+
+define void @bad_use1(...) {
+  call i32 () @llvm.va_arg_pack_len()
+  call void (...) @vaargs() inline_va_arg_pack
+  ret void
+  ; CHECK: call to @llvm.va_arg_pack_len can only appear in a function withavailable_externally linkage
+  ; CHECK: inline_va_arg_pack must be used in an available_externally function
+}
+
+define available_externally void @bad_use2() {
+  call i32 () @llvm.va_arg_pack_len()
+  call void (...) @vaargs() inline_va_arg_pack
+  ; CHECK: call to @llvm.va_arg_pack_len can only appear in a variadic function
+  ; CHECK: inline_va_arg_pack can only appear in a variadic function
+  ret void
+}
+
+define available_externally void @good_use(...) {
+  call i32 () @llvm.va_arg_pack_len()
+  call void (...) @vaargs() inline_va_arg_pack
+  ; CHECK-NOT: @llvm.va_arg_pack_len
+  ; CHECK-NOT: inline_va_arg_pack
+  ret void
+}
+
+declare i32 @llvm.va_arg_pack_len()
+declare void @vaargs(...)