[llvm-commits] [PATCH] Add va_argpack intrinsics
Chandler Carruth
chandlerc at google.com
Tue May 15 09:16:08 PDT 2012
On Tue, May 15, 2012 at 9:08 AM, Benjamin Kramer
<benny.kra at googlemail.com>wrote:
> This will be used to implement __builtin_va_arg_pack[len] in clang
> (PR7219).
>
Awesome, thanks for working on this...
> Since this is used to model a pretty crazy gcc extension (used by glibc)
> we have
> to expand the intrinsics in the inliner. When it replaces a vararg call
> site with
> a copy of the function another pass over all instructions is performed
> which
> replaces any pack intrinsics with the values from the call site. There are
> some
> rather narrow requirements where the intrinsics can occur, documented in
> LangRef
> and enforced by the verifier.
>
> This patch has two other side effects:
> - DAE is not allowed to remove varargs if a va_pack intrinsic is used.
> - Inliner can now inline vararg functions iff they don't use va_start.
> This can
> happen if DAE doesn't remove the varargs because there is a va_pack
> intrinsic
> or if the AlwaysInliner tries to inline the function.
>
I'm pretty OK wit this strategy. It actually has some other interesting
artifacts I'd like to explore later. Specifically, I think we can
generalize this to allow inlining of varargs functions when the va_start is
in dead code...
Comments on the patch:
-------
diff --git a/docs/LangRef.html b/docs/LangRef.html
index 8328def..2c11366 100644
--- a/docs/LangRef.html
+++ b/docs/LangRef.html
@@ -224,6 +224,8 @@
<li><a href="#int_va_start">'<tt>llvm.va_start</tt>'
Intrinsic</a></li>
<li><a href="#int_va_end">'<tt>llvm.va_end</tt>'
Intrinsic</a></li>
<li><a href="#int_va_copy">'<tt>llvm.va_copy</tt>'
Intrinsic</a></li>
+ <li><a href="#int_va_pack">'<tt>llvm.va_pack</tt>'
Intrinsic</a></li>
+ <li><a href="#int_va_packlen">'<tt>llvm.va_packlen</tt>'
Intrinsic</a></li>
'va_pack_len' would be more consistent with the GCC builtin... and as with
the other varargs stuff, I think consistency is good here.
@@ -6669,6 +6671,80 @@ declare void @llvm.va_end(i8*)
</div>
+<!--
_______________________________________________________________________ -->
+<h4>
+ <a name="int_va_pack">'<tt>llvm.va_pack</tt>' Intrinsic</a>
+</h4>
+
+<div>
+
+<h5>Syntax:</h5>
+<pre>
+ declare i8* @llvm.va_pack()
+</pre>
+
+<h5>Overview:</h5>
+<p>The '<tt>llvm.va_pack</tt>' intrinsic expands varargs for a function
call if
+the caller is also a vararg function and is inlined. It can be used to
implement
+functionality like GCC's <tt>__builtin_va_arg_pack</tt>.</p>
+
+<h5>Semantics:</h5>
+<p>The '<tt>llvm.va_pack</tt>' intrinsic may only occur as the last
argument of
+a vararg function call. The calling function must have local or
+<tt>available_externally</tt> linkage so it is never emitted
standalone.</p>
It must also be marked 'always_inline', and its presence requires that the
always inliner is run in order to generate correct code. I think this is
important to require in the semantic model, as this intrinsic has *no*
meaning unless inlining occurs.
diff --git a/lib/Transforms/Utils/InlineFunction.cpp
b/lib/Transforms/Utils/InlineFunction.cpp
index d2b167a..32c5a17 100644
--- a/lib/Transforms/Utils/InlineFunction.cpp
+++ b/lib/Transforms/Utils/InlineFunction.cpp
@@ -477,6 +477,84 @@ static void fixupLineNumbers(Function *Fn,
Function::iterator FI,
}
}
+// Expand va_pack and va_packlen intrinsics between FI and FE with the
+// arguments from CS.
+static void fixupArgPackInstrinsics(CallSite CS,
+ Function::iterator FI,
+ Function::iterator FE) {
+ if (!CS.getCalledFunction()->isVarArg())
+ return;
+
+ // Get the number of varargs in the call.
+ unsigned NumNonVarArgs = CS.getCalledFunction()->arg_size();
+ unsigned NumVarArgs = CS.arg_size() - NumNonVarArgs;
+
+ for (; FI != FE; ++FI)
+ for (BasicBlock::iterator BI = FI->begin(), BE = FI->end(); BI != BE;)
+ if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(BI++)) {
+ switch (II->getIntrinsicID()) {
+ default: break;
+ case Intrinsic::vapack:
I would hoist each case's logic into a helper function --
'for->for->if->switch->for' is too many levels of control for me in a
single function. =]
+ for (Instruction::use_iterator UI = II->use_begin(),
+ UE = II->use_end(); UI != UE; ++UI) {
+ CallSite PackUser(*UI);
+
+ // Create a new list of arguments. First the arguments from
the call
+ // up to the va_pack intrinsic. Then the remaining arguments
from
+ // the call site.
+ std::vector<Value *> NewArgs;
+ NewArgs.insert(NewArgs.end(), PackUser.arg_begin(),
+ llvm::prior(PackUser.arg_end()));
Just construct the vector directly from these?
+ NewArgs.insert(NewArgs.end(), CS.arg_begin() + NumNonVarArgs,
+ CS.arg_end());
+
+ // Now Create a call/invoke with the new arguments.
+ Instruction *New;
+ if (InvokeInst
*II=dyn_cast<InvokeInst>(PackUser.getInstruction()))
+ New = InvokeInst::Create(II->getCalledFunction(),
+ II->getNormalDest(),
II->getUnwindDest(),
+ NewArgs, II->getName(), II);
+ else
+ New = CallInst::Create(PackUser.getCalledFunction(), NewArgs,
+ PackUser->getName(),
+ PackUser.getInstruction());
+
+ // And remove the old call.
+ if (&*BI == PackUser.getInstruction())
+ BI = New;
+ PackUser->replaceAllUsesWith(New);
+ PackUser->eraseFromParent();
+ }
+ // At this point all uses are replaced, remove the intrinsic.
+ II->eraseFromParent();
+ break;
+ case Intrinsic::vapacklen: {
+ ConstantInt *NumVarArgsCI =
+ ConstantInt::get(Type::getInt64Ty(CS.getType()->getContext()),
+ NumVarArgs);
+ II->replaceAllUsesWith(NumVarArgsCI);
+ II->eraseFromParent();
+ break;
+ }
+ }
+ }
+}
+
+// We can still inline a var arg function if the variable arguments aren't
+// actually used. Look explicitly for the vastart intrinsic.
Can you add a fixme here? We shouldn't be doing this in an isolated
for-loop inside the inliner.
I'd like to have inline cost (both the always-inliner and the normal one)
filter inlining when varargs are live in the function, and have the inliner
actually just assert if it encounters one of these intrinsics.... But I
don't think this needs to happen in this patch.
+static bool usesVarArgs(const Function *CalledFunc) {
+ if (CalledFunc->isVarArg())
+ for (Function::const_iterator FI = CalledFunc->begin(),
+ FE = CalledFunc->end(); FI != FE; ++FI)
+ for (BasicBlock::const_iterator BI = FI->begin(), BE = FI->end();
+ BI != BE; ++BI)
+ if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(BI))
+ if (II->getIntrinsicID() == Intrinsic::vastart)
+ return true;
+
+ return false;
+}
+
/// InlineFunction - This function inlines the called function into the
basic
/// block of the caller. This returns false if it is not possible to
inline
/// this call. The program is still in a well defined state if this occurs
@@ -498,7 +576,7 @@ bool llvm::InlineFunction(CallSite CS,
InlineFunctionInfo &IFI,
const Function *CalledFunc = CS.getCalledFunction();
if (CalledFunc == 0 || // Can't inline external function or
indirect
CalledFunc->isDeclaration() || // call, or call to a vararg function!
- CalledFunc->getFunctionType()->isVarArg()) return false;
+ usesVarArgs(CalledFunc)) return false;
// If the call to the callee is not a tail call, we must clear the 'tail'
// flags on any calls that we inline.
@@ -568,9 +646,6 @@ bool llvm::InlineFunction(CallSite CS,
InlineFunctionInfo &IFI,
{ // Scope to destroy VMap after cloning.
ValueToValueMapTy VMap;
- assert(CalledFunc->arg_size() == CS.arg_size() &&
- "No varargs calls can be inlined!");
-
// Calculate the vector of arguments to pass into the function cloner,
which
// matches up the formal to the actual argument values.
CallSite::arg_iterator AI = CS.arg_begin();
@@ -721,6 +796,9 @@ bool llvm::InlineFunction(CallSite CS,
InlineFunctionInfo &IFI,
// return instruction, we splice the body of the inlined callee directly
into
// the calling basic block.
if (Returns.size() == 1 && std::distance(FirstNewBlock, Caller->end())
== 1) {
+ // Expand va_pack intrinsics.
+ fixupArgPackInstrinsics(TheCall, FirstNewBlock, Caller->end());
+
// Move all of the instructions right before the call.
OrigBB->getInstList().splice(TheCall, FirstNewBlock->getInstList(),
FirstNewBlock->begin(),
FirstNewBlock->end());
@@ -785,6 +863,8 @@ bool llvm::InlineFunction(CallSite CS,
InlineFunctionInfo &IFI,
"splitBasicBlock broken!");
Br->setOperand(0, FirstNewBlock);
+ // Expand va_pack intrinsics.
+ fixupArgPackInstrinsics(TheCall, FirstNewBlock, Caller->end());
// Now that the function is correct, make it a little bit nicer. In
// particular, move the basic blocks inserted from the end of the
function
diff --git a/lib/VMCore/Verifier.cpp b/lib/VMCore/Verifier.cpp
index 47baef3..cc12140 100644
--- a/lib/VMCore/Verifier.cpp
+++ b/lib/VMCore/Verifier.cpp
@@ -1769,6 +1769,24 @@ void
Verifier::visitIntrinsicFunctionCall(Intrinsic::ID ID, CallInst &CI) {
Assert1(isa<ConstantInt>(CI.getArgOperand(1)),
"llvm.invariant.end parameter #2 must be a constant integer",
&CI);
break;
+ case Intrinsic::vapack:
+ for (Value::use_iterator UI = CI.use_begin(), UE = CI.use_end(); UI !=
UE;
+ ++UI) {
+ Assert1(isa<CallInst>(*UI) || isa<InvokeInst>(*UI),
+ "llvm.vapack is only allowed as an argument", &CI);
"va_pack"... missing '_' here and below.
+ Assert1(CallSite(*UI).getCalledFunction()->isVarArg(),
+ "llvm.vapack must be used as a vararg argument", &CI);
+ Assert1(*llvm::prior(CallSite(*UI).arg_end()) == &CI,
+ "llvm.vapack must be the last argument in a call", &CI);
+ }
+ // FALLTHROUGH
+ case Intrinsic::vapacklen:
+ Function *F = CI.getParent()->getParent();
+ Assert1(F->isVarArg(),
+ "llvm.vapack must not be called outside of vararg functions",
&CI);
+ Assert1(F->hasLocalLinkage() || F->hasAvailableExternallyLinkage(),
+ "Function containing llvm.vapack intrinsics must be inlined",
&CI);
We should also verify that the caller is always_inline.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120515/4a7ae29e/attachment.html>
More information about the llvm-commits
mailing list