From 44b3a0b4115ad000540370897b30dc01f406bf0f Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Tue, 26 Aug 2014 00:33:28 +0000 Subject: [PATCH] Declare that musttail calls in variadic functions forward the ellipsis Summary: There is no functionality change here except in the way we assemble and dump musttail calls in variadic functions. There's really no need to separate out the bits for musttail and "is forwarding varargs" on call instructions. A musttail call by definition has to forward the ellipsis or it would fail verification. Reviewers: chandlerc, nlewycky Subscribers: llvm-commits Differential Revision: http://reviews.llvm.org/D4892 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@216423 91177308-0d34-0410-b5e6-96231b3b80d8 --- docs/LangRef.rst | 3 +++ lib/AsmParser/LLParser.cpp | 21 +++++++++++++++++++-- lib/AsmParser/LLParser.h | 4 +++- lib/IR/AsmWriter.cpp | 8 ++++++++ test/Assembler/musttail-invalid-1.ll | 14 ++++++++++++++ test/Assembler/musttail-invalid-2.ll | 13 +++++++++++++ test/Assembler/musttail.ll | 14 ++++++++++++++ test/Transforms/Inline/inline-musttail-varargs.ll | 22 ++++++++++++++++++++++ 8 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 test/Assembler/musttail-invalid-1.ll create mode 100644 test/Assembler/musttail-invalid-2.ll create mode 100644 test/Assembler/musttail.ll create mode 100644 test/Transforms/Inline/inline-musttail-varargs.ll diff --git a/docs/LangRef.rst b/docs/LangRef.rst index a310240804d..8e574dfd7af 100644 --- a/docs/LangRef.rst +++ b/docs/LangRef.rst @@ -6591,6 +6591,9 @@ This instruction requires several arguments: - The calling conventions of the caller and callee must match. - All ABI-impacting function attributes, such as sret, byval, inreg, returned, and inalloca, must match. + - The callee must be varargs iff the caller is varargs. Bitcasting a + non-varargs function to the appropriate varargs type is legal so + long as the non-varargs prefixes obey the other rules. Tail call optimization for calls marked ``tail`` is guaranteed to occur if the following conditions are met: diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp index 177243cf647..7a06a48da76 100644 --- a/lib/AsmParser/LLParser.cpp +++ b/lib/AsmParser/LLParser.cpp @@ -1819,7 +1819,8 @@ bool LLParser::ParseType(Type *&Result, bool AllowVoid) { /// Arg /// ::= Type OptionalAttributes Value OptionalAttributes bool LLParser::ParseParameterList(SmallVectorImpl &ArgList, - PerFunctionState &PFS) { + PerFunctionState &PFS, bool IsMustTailCall, + bool InVarArgsFunc) { if (ParseToken(lltok::lparen, "expected '(' in call")) return true; @@ -1830,6 +1831,17 @@ bool LLParser::ParseParameterList(SmallVectorImpl &ArgList, ParseToken(lltok::comma, "expected ',' in argument list")) return true; + // Parse an ellipsis if this is a musttail call in a variadic function. + if (Lex.getKind() == lltok::dotdotdot) { + const char *Msg = "unexpected ellipsis in argument list for "; + if (!IsMustTailCall) + return TokError(Twine(Msg) + "non-musttail call"); + if (!InVarArgsFunc) + return TokError(Twine(Msg) + "musttail call in non-varargs function"); + Lex.Lex(); // Lex the '...', it is purely for readability. + return ParseToken(lltok::rparen, "expected ')' at end of argument list"); + } + // Parse the argument. LocTy ArgLoc; Type *ArgTy = nullptr; @@ -1846,6 +1858,10 @@ bool LLParser::ParseParameterList(SmallVectorImpl &ArgList, ArgAttrs))); } + if (IsMustTailCall && InVarArgsFunc) + return TokError("expected '...' at end of argument list for musttail call " + "in varargs function"); + Lex.Lex(); // Lex the ')'. return false; } @@ -4214,7 +4230,8 @@ bool LLParser::ParseCall(Instruction *&Inst, PerFunctionState &PFS, ParseOptionalReturnAttrs(RetAttrs) || ParseType(RetType, RetTypeLoc, true /*void allowed*/) || ParseValID(CalleeID) || - ParseParameterList(ArgList, PFS) || + ParseParameterList(ArgList, PFS, TCK == CallInst::TCK_MustTail, + PFS.getFunction().isVarArg()) || ParseFnAttributeValuePairs(FnAttrs, FwdRefAttrGrps, false, BuiltinLoc)) return true; diff --git a/lib/AsmParser/LLParser.h b/lib/AsmParser/LLParser.h index 3936c6e8731..b7d8f21442b 100644 --- a/lib/AsmParser/LLParser.h +++ b/lib/AsmParser/LLParser.h @@ -372,7 +372,9 @@ namespace llvm { : Loc(loc), V(v), Attrs(attrs) {} }; bool ParseParameterList(SmallVectorImpl &ArgList, - PerFunctionState &PFS); + PerFunctionState &PFS, + bool IsMustTailCall = false, + bool InVarArgsFunc = false); // Constant Parsing. bool ParseValID(ValID &ID, PerFunctionState *PFS = nullptr); diff --git a/lib/IR/AsmWriter.cpp b/lib/IR/AsmWriter.cpp index 8aebc4396b6..ebc78e0517f 100644 --- a/lib/IR/AsmWriter.cpp +++ b/lib/IR/AsmWriter.cpp @@ -2174,6 +2174,14 @@ void AssemblyWriter::printInstruction(const Instruction &I) { Out << ", "; writeParamOperand(CI->getArgOperand(op), PAL, op + 1); } + + // Emit an ellipsis if this is a musttail call in a vararg function. This + // is only to aid readability, musttail calls forward varargs by default. + if (CI->isMustTailCall() && CI->getParent() && + CI->getParent()->getParent() && + CI->getParent()->getParent()->isVarArg()) + Out << ", ..."; + Out << ')'; if (PAL.hasAttributes(AttributeSet::FunctionIndex)) Out << " #" << Machine.getAttributeGroupSlot(PAL.getFnAttributes()); diff --git a/test/Assembler/musttail-invalid-1.ll b/test/Assembler/musttail-invalid-1.ll new file mode 100644 index 00000000000..b123a9151d3 --- /dev/null +++ b/test/Assembler/musttail-invalid-1.ll @@ -0,0 +1,14 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +; Check the error message on using ", ..." when we can't actually forward +; varargs. + +%struct.A = type { i32 } + +declare i8* @f(i8*, ...) + +define i8* @f_thunk(i8* %this) { + %rv = musttail call i8* (i8*, ...)* @f(i8* %this, ...) +; CHECK: error: unexpected ellipsis in argument list for musttail call in non-varargs function + ret i8* %rv +} diff --git a/test/Assembler/musttail-invalid-2.ll b/test/Assembler/musttail-invalid-2.ll new file mode 100644 index 00000000000..3bcb51fc485 --- /dev/null +++ b/test/Assembler/musttail-invalid-2.ll @@ -0,0 +1,13 @@ +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +; Check the error message on skipping ", ..." at the end of a musttail call argument list. + +%struct.A = type { i32 } + +declare i8* @f(i8*, ...) + +define i8* @f_thunk(i8* %this, ...) { + %rv = musttail call i8* (i8*, ...)* @f(i8* %this) +; CHECK: error: expected '...' at end of argument list for musttail call in varargs function + ret i8* %rv +} diff --git a/test/Assembler/musttail.ll b/test/Assembler/musttail.ll new file mode 100644 index 00000000000..6e2a9b2ef32 --- /dev/null +++ b/test/Assembler/musttail.ll @@ -0,0 +1,14 @@ +; RUN: llvm-as < %s | llvm-dis | FileCheck %s + +; Check that the ellipsis round trips. + +%struct.A = type { i32 } + +declare i8* @f(i8*, ...) + +define i8* @f_thunk(i8* %this, ...) { + %rv = musttail call i8* (i8*, ...)* @f(i8* %this, ...) + ret i8* %rv +} +; CHECK-LABEL: define i8* @f_thunk(i8* %this, ...) +; CHECK: %rv = musttail call i8* (i8*, ...)* @f(i8* %this, ...) diff --git a/test/Transforms/Inline/inline-musttail-varargs.ll b/test/Transforms/Inline/inline-musttail-varargs.ll new file mode 100644 index 00000000000..7a8957488e1 --- /dev/null +++ b/test/Transforms/Inline/inline-musttail-varargs.ll @@ -0,0 +1,22 @@ +; RUN: opt < %s -inline -instcombine -S | FileCheck %s + +; We can't inline this thunk yet, but one day we will be able to. And when we +; do, this test case will be ready. + +declare void @ext_method(i8*, i32) + +define linkonce_odr void @thunk(i8* %this, ...) { + %this_adj = getelementptr i8* %this, i32 4 + musttail call void (i8*, ...)* bitcast (void (i8*, i32)* @ext_method to void (i8*, ...)*)(i8* %this_adj, ...) + ret void +} + +define void @thunk_caller(i8* %p) { + call void (i8*, ...)* @thunk(i8* %p, i32 42) + ret void +} +; CHECK-LABEL: define void @thunk_caller(i8* %p) +; CHECK: call void (i8*, ...)* @thunk(i8* %p, i32 42) + +; FIXME: Inline the thunk. This should be significantly easier than inlining +; general varargs functions. -- 2.11.0