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}; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits