r296024 - [CodeGen] Fix ExtParameterInfo bugs in C++ CodeGen code.
George Burgess IV via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 23 14:07:36 PST 2017
Author: gbiv
Date: Thu Feb 23 16:07:35 2017
New Revision: 296024
URL: http://llvm.org/viewvc/llvm-project?rev=296024&view=rev
Log:
[CodeGen] Fix ExtParameterInfo bugs in C++ CodeGen code.
This patch makes use of the prefix/suffix ABI argument distinction that
was introduced in r295870, so that we now emit ExtParameterInfo at the
correct offset for member calls that have added ABI arguments. I don't
see a good way to test the generated param info, since we don't actually
seem to use it in CGFunctionInfo outside of Swift. Any
suggestions/thoughts for how to better test this are welcome. :)
This patch also fixes a small bug with inheriting constructors: if we
decide not to pass args into an base class ctor, we would still
generate ExtParameterInfo as though we did. The added test-case is for
that behavior.
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/CGVTables.cpp
cfe/trunk/lib/CodeGen/CodeGenTypes.h
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/test/CodeGenObjCXX/arc-attrs-abi.mm
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=296024&r1=296023&r2=296024&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Feb 23 16:07:35 2017
@@ -362,18 +362,31 @@ getExtParameterInfosForCall(const Functi
}
/// Arrange a call to a C++ method, passing the given arguments.
+///
+/// ExtraPrefixArgs is the number of ABI-specific args passed after the `this`
+/// parameter.
+/// ExtraSuffixArgs is the number of ABI-specific args passed at the end of
+/// args.
+/// PassProtoArgs indicates whether `args` has args for the parameters in the
+/// given CXXConstructorDecl.
const CGFunctionInfo &
CodeGenTypes::arrangeCXXConstructorCall(const CallArgList &args,
const CXXConstructorDecl *D,
CXXCtorType CtorKind,
- unsigned ExtraArgs) {
+ unsigned ExtraPrefixArgs,
+ unsigned ExtraSuffixArgs,
+ bool PassProtoArgs) {
// FIXME: Kill copy.
SmallVector<CanQualType, 16> ArgTypes;
for (const auto &Arg : args)
ArgTypes.push_back(Context.getCanonicalParamType(Arg.Ty));
+ // +1 for implicit this, which should always be args[0].
+ unsigned TotalPrefixArgs = 1 + ExtraPrefixArgs;
+
CanQual<FunctionProtoType> FPT = GetFormalType(D);
- RequiredArgs Required = RequiredArgs::forPrototypePlus(FPT, 1 + ExtraArgs, D);
+ RequiredArgs Required =
+ RequiredArgs::forPrototypePlus(FPT, TotalPrefixArgs + ExtraSuffixArgs, D);
GlobalDecl GD(D, CtorKind);
CanQualType ResultType = TheCXXABI.HasThisReturn(GD)
? ArgTypes.front()
@@ -382,8 +395,14 @@ CodeGenTypes::arrangeCXXConstructorCall(
: Context.VoidTy;
FunctionType::ExtInfo Info = FPT->getExtInfo();
- auto ParamInfos = getExtParameterInfosForCall(FPT.getTypePtr(), 1 + ExtraArgs,
- ArgTypes.size());
+ llvm::SmallVector<FunctionProtoType::ExtParameterInfo, 16> ParamInfos;
+ // If the prototype args are elided, we should only have ABI-specific args,
+ // which never have param info.
+ if (PassProtoArgs && FPT->hasExtParameterInfos()) {
+ // ABI-specific suffix arguments are treated the same as variadic arguments.
+ addExtParameterInfosForCall(ParamInfos, FPT.getTypePtr(), TotalPrefixArgs,
+ ArgTypes.size());
+ }
return arrangeLLVMFunctionInfo(ResultType, /*instanceMethod=*/true,
/*chainCall=*/false, ArgTypes, Info,
ParamInfos, Required);
@@ -627,15 +646,20 @@ CodeGenTypes::arrangeBuiltinFunctionDecl
}
/// Arrange a call to a C++ method, passing the given arguments.
+///
+/// numPrefixArgs is the number of ABI-specific prefix arguments we have. It
+/// does not count `this`.
const CGFunctionInfo &
CodeGenTypes::arrangeCXXMethodCall(const CallArgList &args,
const FunctionProtoType *proto,
- RequiredArgs required) {
- unsigned numRequiredArgs =
- (proto->isVariadic() ? required.getNumRequiredArgs() : args.size());
- unsigned numPrefixArgs = numRequiredArgs - proto->getNumParams();
+ RequiredArgs required,
+ unsigned numPrefixArgs) {
+ assert(numPrefixArgs + 1 <= args.size() &&
+ "Emitting a call with less args than the required prefix?");
+ // Add one to account for `this`. It's a bit awkward here, but we don't count
+ // `this` in similar places elsewhere.
auto paramInfos =
- getExtParameterInfosForCall(proto, numPrefixArgs, args.size());
+ getExtParameterInfosForCall(proto, numPrefixArgs + 1, args.size());
// FIXME: Kill copy.
auto argTypes = getArgTypesForCall(Context, args);
Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=296024&r1=296023&r2=296024&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Thu Feb 23 16:07:35 2017
@@ -1979,7 +1979,7 @@ static bool canEmitDelegateCallArgs(Code
// Likewise if they're inalloca.
const CGFunctionInfo &Info =
- CGF.CGM.getTypes().arrangeCXXConstructorCall(Args, Ctor, Type, 0);
+ CGF.CGM.getTypes().arrangeCXXConstructorCall(Args, Ctor, Type, 0, 0);
if (Info.usesInAlloca())
return false;
}
@@ -2021,10 +2021,11 @@ void CodeGenFunction::EmitCXXConstructor
return;
}
+ bool PassPrototypeArgs = true;
// Check whether we can actually emit the constructor before trying to do so.
if (auto Inherited = D->getInheritedConstructor()) {
- if (getTypes().inheritingCtorHasParams(Inherited, Type) &&
- !canEmitDelegateCallArgs(*this, D, Type, Args)) {
+ PassPrototypeArgs = getTypes().inheritingCtorHasParams(Inherited, Type);
+ if (PassPrototypeArgs && !canEmitDelegateCallArgs(*this, D, Type, Args)) {
EmitInlinedInheritingCXXConstructorCall(D, Type, ForVirtualBase,
Delegating, Args);
return;
@@ -2040,7 +2041,7 @@ void CodeGenFunction::EmitCXXConstructor
llvm::Constant *CalleePtr =
CGM.getAddrOfCXXStructor(D, getFromCtorType(Type));
const CGFunctionInfo &Info = CGM.getTypes().arrangeCXXConstructorCall(
- Args, D, Type, ExtraArgs.Prefix + ExtraArgs.Suffix);
+ Args, D, Type, ExtraArgs.Prefix, ExtraArgs.Suffix, PassPrototypeArgs);
CGCallee Callee = CGCallee::forDirect(CalleePtr, D);
EmitCall(Info, Callee, ReturnValueSlot(), Args);
Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=296024&r1=296023&r2=296024&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Thu Feb 23 16:07:35 2017
@@ -24,7 +24,15 @@
using namespace clang;
using namespace CodeGen;
-static RequiredArgs
+namespace {
+struct MemberCallInfo {
+ RequiredArgs ReqArgs;
+ // Number of prefix arguments for the call. Ignores the `this` pointer.
+ unsigned PrefixSize;
+};
+}
+
+static MemberCallInfo
commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, const CXXMethodDecl *MD,
llvm::Value *This, llvm::Value *ImplicitParam,
QualType ImplicitParamTy, const CallExpr *CE,
@@ -48,6 +56,7 @@ commonEmitCXXMemberOrOperatorCall(CodeGe
const FunctionProtoType *FPT = MD->getType()->castAs<FunctionProtoType>();
RequiredArgs required = RequiredArgs::forPrototypePlus(FPT, Args.size(), MD);
+ unsigned PrefixSize = Args.size() - 1;
// And the rest of the call args.
if (RtlArgs) {
@@ -65,7 +74,7 @@ commonEmitCXXMemberOrOperatorCall(CodeGe
FPT->getNumParams() == 0 &&
"No CallExpr specified for function with non-zero number of arguments");
}
- return required;
+ return {required, PrefixSize};
}
RValue CodeGenFunction::EmitCXXMemberOrOperatorCall(
@@ -75,9 +84,10 @@ RValue CodeGenFunction::EmitCXXMemberOrO
const CallExpr *CE, CallArgList *RtlArgs) {
const FunctionProtoType *FPT = MD->getType()->castAs<FunctionProtoType>();
CallArgList Args;
- RequiredArgs required = commonEmitCXXMemberOrOperatorCall(
+ MemberCallInfo CallInfo = commonEmitCXXMemberOrOperatorCall(
*this, MD, This, ImplicitParam, ImplicitParamTy, CE, Args, RtlArgs);
- auto &FnInfo = CGM.getTypes().arrangeCXXMethodCall(Args, FPT, required);
+ auto &FnInfo = CGM.getTypes().arrangeCXXMethodCall(
+ Args, FPT, CallInfo.ReqArgs, CallInfo.PrefixSize);
return EmitCall(FnInfo, Callee, ReturnValue, Args);
}
@@ -425,7 +435,8 @@ CodeGenFunction::EmitCXXMemberPointerCal
// And the rest of the call args
EmitCallArgs(Args, FPT, E->arguments());
- return EmitCall(CGM.getTypes().arrangeCXXMethodCall(Args, FPT, required),
+ return EmitCall(CGM.getTypes().arrangeCXXMethodCall(Args, FPT, required,
+ /*PrefixSize=*/0),
Callee, ReturnValue, Args);
}
Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=296024&r1=296023&r2=296024&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Thu Feb 23 16:07:35 2017
@@ -284,6 +284,7 @@ void CodeGenFunction::EmitCallAndReturnF
if (isa<CXXDestructorDecl>(MD))
CGM.getCXXABI().adjustCallArgsForDestructorThunk(*this, CurGD, CallArgs);
+ unsigned PrefixArgs = CallArgs.size() - 1;
// Add the rest of the arguments.
for (const ParmVarDecl *PD : MD->parameters())
EmitDelegateCallArg(CallArgs, PD, SourceLocation());
@@ -292,7 +293,7 @@ void CodeGenFunction::EmitCallAndReturnF
#ifndef NDEBUG
const CGFunctionInfo &CallFnInfo = CGM.getTypes().arrangeCXXMethodCall(
- CallArgs, FPT, RequiredArgs::forPrototypePlus(FPT, 1, MD));
+ CallArgs, FPT, RequiredArgs::forPrototypePlus(FPT, 1, MD), PrefixArgs);
assert(CallFnInfo.getRegParm() == CurFnInfo->getRegParm() &&
CallFnInfo.isNoReturn() == CurFnInfo->isNoReturn() &&
CallFnInfo.getCallingConvention() == CurFnInfo->getCallingConvention());
Modified: cfe/trunk/lib/CodeGen/CodeGenTypes.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenTypes.h?rev=296024&r1=296023&r2=296024&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenTypes.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenTypes.h Thu Feb 23 16:07:35 2017
@@ -303,11 +303,14 @@ public:
const CGFunctionInfo &arrangeCXXConstructorCall(const CallArgList &Args,
const CXXConstructorDecl *D,
CXXCtorType CtorKind,
- unsigned ExtraArgs);
+ unsigned ExtraPrefixArgs,
+ unsigned ExtraSuffixArgs,
+ bool PassProtoArgs = true);
const CGFunctionInfo &arrangeCXXMethodCall(const CallArgList &args,
const FunctionProtoType *type,
- RequiredArgs required);
+ RequiredArgs required,
+ unsigned numPrefixArgs);
const CGFunctionInfo &arrangeMSMemberPointerThunk(const CXXMethodDecl *MD);
const CGFunctionInfo &arrangeMSCtorClosure(const CXXConstructorDecl *CD,
CXXCtorType CT);
Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=296024&r1=296023&r2=296024&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Thu Feb 23 16:07:35 2017
@@ -3934,7 +3934,7 @@ MicrosoftCXXABI::getAddrOfCXXCtorClosure
CGM.getAddrOfCXXStructor(CD, StructorType::Complete);
CGCallee Callee = CGCallee::forDirect(CalleePtr, CD);
const CGFunctionInfo &CalleeInfo = CGM.getTypes().arrangeCXXConstructorCall(
- Args, CD, Ctor_Complete, ExtraArgs.Prefix + ExtraArgs.Suffix);
+ Args, CD, Ctor_Complete, ExtraArgs.Prefix, ExtraArgs.Suffix);
CGF.EmitCall(CalleeInfo, Callee, ReturnValueSlot(), Args);
Cleanups.ForceCleanup();
Modified: cfe/trunk/test/CodeGenObjCXX/arc-attrs-abi.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/arc-attrs-abi.mm?rev=296024&r1=296023&r2=296024&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjCXX/arc-attrs-abi.mm (original)
+++ cfe/trunk/test/CodeGenObjCXX/arc-attrs-abi.mm Thu Feb 23 16:07:35 2017
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -triple x86_64-apple -emit-llvm -fobjc-arc -o - %s
-// RUN: %clang_cc1 -triple x86_64-windows -emit-llvm -fobjc-arc -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple -emit-llvm -fobjc-arc -o - %s -std=c++11 | FileCheck %s --check-prefix=ITANIUM
+// RUN: %clang_cc1 -triple x86_64-windows -emit-llvm -fobjc-arc -o - %s -std=c++11
//
-// Test caess where we weren't properly adding parameter infos declarations,
-// which caused assertions to fire. Hence, no CHECKs.
+// Test cases where we weren't properly adding extended parameter info, which
+// caused assertions to fire. Hence, minimal CHECKs.
struct VirtualBase {
VirtualBase(__attribute__((ns_consumed)) id x);
@@ -13,3 +13,25 @@ struct WithVirtualBase : virtual Virtual
WithVirtualBase::WithVirtualBase(__attribute__((ns_consumed)) id x)
: VirtualBase(x) {}
+
+
+struct VirtualBase2 {
+ VirtualBase2(__attribute__((ns_consumed)) id x, void *y);
+};
+
+// In this case, we don't actually end up passing the `id` param from
+// WithVirtualBaseLast's ctor to WithVirtualBaseMid's. So, we shouldn't emit
+// ext param info for `id` to `Mid`. Itanium-only check since MSABI seems to
+// emit the construction code inline.
+struct WithVirtualBaseMid : virtual VirtualBase2 {
+ // Ensure we only pass in `this` and a vtable. Otherwise this test is useless.
+ // ITANIUM: define {{.*}} void @_ZN18WithVirtualBaseMidCI212VirtualBase2EP11objc_objectPv({{.*}}, {{.*}})
+ using VirtualBase2::VirtualBase2;
+};
+struct WithVirtualBaseLast : WithVirtualBaseMid {
+ using WithVirtualBaseMid::WithVirtualBaseMid;
+};
+
+void callLast(__attribute__((ns_consumed)) id x) {
+ WithVirtualBaseLast{x, (void*)0};
+}
More information about the cfe-commits
mailing list