From bcaedaed309ce453a992fdeef4a4c908cc7d9dfb Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Wed, 30 Jun 2010 19:14:05 +0000 Subject: [PATCH] Reapply: r107173, "fix PR7519: after thrashing around and remembering how all this stuff" r107216, "fix PR7523, which was caused by the ABI code calling ConvertType instead" This includes a fix to make ConvertTypeForMem handle the "recursive" case, and call it as such when lowering function types which have an indirect result. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@107310 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGCall.cpp | 57 +++++++++++++++++---------- lib/CodeGen/CodeGenModule.cpp | 2 + lib/CodeGen/CodeGenTypes.cpp | 59 ++++++++++++++++------------ lib/CodeGen/CodeGenTypes.h | 24 +++++++---- lib/CodeGen/TargetInfo.cpp | 3 +- test/CodeGen/decl.c | 13 ++++++ test/CodeGenCXX/x86_64-arguments.cpp | 15 +++++++ 7 files changed, 117 insertions(+), 56 deletions(-) diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp index 484eee9587..24a64cfa44 100644 --- a/lib/CodeGen/CGCall.cpp +++ b/lib/CodeGen/CGCall.cpp @@ -61,28 +61,31 @@ static CanQualType GetReturnType(QualType RetTy) { } const CGFunctionInfo & -CodeGenTypes::getFunctionInfo(CanQual FTNP) { +CodeGenTypes::getFunctionInfo(CanQual FTNP, + bool IsRecursive) { return getFunctionInfo(FTNP->getResultType().getUnqualifiedType(), llvm::SmallVector(), - FTNP->getExtInfo()); + FTNP->getExtInfo(), IsRecursive); } /// \param Args - contains any initial parameters besides those /// in the formal type static const CGFunctionInfo &getFunctionInfo(CodeGenTypes &CGT, llvm::SmallVectorImpl &ArgTys, - CanQual FTP) { + CanQual FTP, + bool IsRecursive = false) { // FIXME: Kill copy. for (unsigned i = 0, e = FTP->getNumArgs(); i != e; ++i) ArgTys.push_back(FTP->getArgType(i)); CanQualType ResTy = FTP->getResultType().getUnqualifiedType(); - return CGT.getFunctionInfo(ResTy, ArgTys, FTP->getExtInfo()); + return CGT.getFunctionInfo(ResTy, ArgTys, FTP->getExtInfo(), IsRecursive); } const CGFunctionInfo & -CodeGenTypes::getFunctionInfo(CanQual FTP) { +CodeGenTypes::getFunctionInfo(CanQual FTP, + bool IsRecursive) { llvm::SmallVector ArgTys; - return ::getFunctionInfo(*this, ArgTys, FTP); + return ::getFunctionInfo(*this, ArgTys, FTP, IsRecursive); } static CallingConv getCallingConventionForDecl(const Decl *D) { @@ -215,7 +218,8 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(QualType ResTy, const CGFunctionInfo &CodeGenTypes::getFunctionInfo(CanQualType ResTy, const llvm::SmallVectorImpl &ArgTys, - const FunctionType::ExtInfo &Info) { + const FunctionType::ExtInfo &Info, + bool IsRecursive) { #ifndef NDEBUG for (llvm::SmallVectorImpl::const_iterator I = ArgTys.begin(), E = ArgTys.end(); I != E; ++I) @@ -243,8 +247,17 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(CanQualType ResTy, // various situations, pass it in. llvm::SmallVector PreferredArgTypes; for (llvm::SmallVectorImpl::const_iterator - I = ArgTys.begin(), E = ArgTys.end(); I != E; ++I) - PreferredArgTypes.push_back(ConvertType(*I)); + I = ArgTys.begin(), E = ArgTys.end(); I != E; ++I) { + // If this is being called from the guts of the ConvertType loop, make sure + // to call ConvertTypeRecursive so we don't get into issues with cyclic + // pointer type structures. + const llvm::Type *ArgType; + if (IsRecursive) + ArgType = ConvertTypeRecursive(*I); + else + ArgType = ConvertType(*I); + PreferredArgTypes.push_back(ArgType); + } // Compute ABI information. getABIInfo().computeInfo(*FI, getContext(), TheModule.getContext(), @@ -274,7 +287,8 @@ CGFunctionInfo::CGFunctionInfo(unsigned _CallingConvention, /***/ void CodeGenTypes::GetExpandedTypes(QualType Ty, - std::vector &ArgTys) { + std::vector &ArgTys, + bool IsRecursive) { const RecordType *RT = Ty->getAsStructureType(); assert(RT && "Can only expand structure types."); const RecordDecl *RD = RT->getDecl(); @@ -289,9 +303,9 @@ void CodeGenTypes::GetExpandedTypes(QualType Ty, QualType FT = FD->getType(); if (CodeGenFunction::hasAggregateLLVMType(FT)) { - GetExpandedTypes(FT, ArgTys); + GetExpandedTypes(FT, ArgTys, IsRecursive); } else { - ArgTys.push_back(ConvertType(FT)); + ArgTys.push_back(ConvertType(FT, IsRecursive)); } } } @@ -554,11 +568,12 @@ const llvm::FunctionType *CodeGenTypes::GetFunctionType(GlobalDecl GD) { cast(GD.getDecl())->getType()->getAs()) Variadic = FPT->isVariadic(); - return GetFunctionType(FI, Variadic); + return GetFunctionType(FI, Variadic, false); } const llvm::FunctionType * -CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI, bool IsVariadic) { +CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI, bool IsVariadic, + bool IsRecursive) { std::vector ArgTys; const llvm::Type *ResultType = 0; @@ -571,13 +586,13 @@ CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI, bool IsVariadic) { case ABIArgInfo::Extend: case ABIArgInfo::Direct: - ResultType = ConvertType(RetTy); + ResultType = ConvertType(RetTy, IsRecursive); break; case ABIArgInfo::Indirect: { assert(!RetAI.getIndirectAlign() && "Align unused on indirect return."); ResultType = llvm::Type::getVoidTy(getLLVMContext()); - const llvm::Type *STy = ConvertType(RetTy); + const llvm::Type *STy = ConvertType(RetTy, IsRecursive); ArgTys.push_back(llvm::PointerType::get(STy, RetTy.getAddressSpace())); break; } @@ -615,18 +630,18 @@ CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI, bool IsVariadic) { case ABIArgInfo::Indirect: { // indirect arguments are always on the stack, which is addr space #0. - const llvm::Type *LTy = ConvertTypeForMem(it->type); + const llvm::Type *LTy = ConvertTypeForMem(it->type, IsRecursive); ArgTys.push_back(llvm::PointerType::getUnqual(LTy)); break; } case ABIArgInfo::Extend: case ABIArgInfo::Direct: - ArgTys.push_back(ConvertType(it->type)); + ArgTys.push_back(ConvertType(it->type, IsRecursive)); break; case ABIArgInfo::Expand: - GetExpandedTypes(it->type, ArgTys); + GetExpandedTypes(it->type, ArgTys, IsRecursive); break; } } @@ -639,7 +654,7 @@ CodeGenTypes::GetFunctionTypeForVTable(const CXXMethodDecl *MD) { const FunctionProtoType *FPT = MD->getType()->getAs(); if (!VerifyFuncTypeComplete(FPT)) - return GetFunctionType(getFunctionInfo(MD), FPT->isVariadic()); + return GetFunctionType(getFunctionInfo(MD), FPT->isVariadic(), false); return llvm::OpaqueType::get(getLLVMContext()); } @@ -774,7 +789,7 @@ void CodeGenModule::ConstructAttributeList(const CGFunctionInfo &FI, // FIXME: This is rather inefficient. Do we ever actually need to do // anything here? The result should be just reconstructed on the other // side, so extension should be a non-issue. - getTypes().GetExpandedTypes(ParamType, Tys); + getTypes().GetExpandedTypes(ParamType, Tys, false); Index += Tys.size(); continue; } diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp index 6e12e1b904..7984db12d5 100644 --- a/lib/CodeGen/CodeGenModule.cpp +++ b/lib/CodeGen/CodeGenModule.cpp @@ -872,6 +872,7 @@ CodeGenModule::GetOrCreateLLVMFunction(llvm::StringRef MangledName, std::vector(), false); IsIncompleteFunction = true; } + llvm::Function *F = llvm::Function::Create(FTy, llvm::Function::ExternalLinkage, MangledName, &getModule()); @@ -932,6 +933,7 @@ llvm::Constant *CodeGenModule::GetAddrOfFunction(GlobalDecl GD, // If there was no specific requested type, just convert it now. if (!Ty) Ty = getTypes().ConvertType(cast(GD.getDecl())->getType()); + llvm::StringRef MangledName = getMangledName(GD); return GetOrCreateLLVMFunction(MangledName, Ty, GD); } diff --git a/lib/CodeGen/CodeGenTypes.cpp b/lib/CodeGen/CodeGenTypes.cpp index cf23c07394..7876e5aefc 100644 --- a/lib/CodeGen/CodeGenTypes.cpp +++ b/lib/CodeGen/CodeGenTypes.cpp @@ -43,10 +43,15 @@ CodeGenTypes::~CodeGenTypes() { } /// ConvertType - Convert the specified type to its LLVM form. -const llvm::Type *CodeGenTypes::ConvertType(QualType T) { - llvm::PATypeHolder Result = ConvertTypeRecursive(T); +const llvm::Type *CodeGenTypes::ConvertType(QualType T, bool IsRecursive) { + const llvm::Type *RawResult = ConvertTypeRecursive(T); + + if (IsRecursive || PointersToResolve.empty()) + return RawResult; - // Any pointers that were converted defered evaluation of their pointee type, + llvm::PATypeHolder Result = RawResult; + + // Any pointers that were converted deferred evaluation of their pointee type, // creating an opaque type instead. This is in order to avoid problems with // circular types. Loop through all these defered pointees, if any, and // resolve them now. @@ -80,21 +85,12 @@ const llvm::Type *CodeGenTypes::ConvertTypeRecursive(QualType T) { return ResultType; } -const llvm::Type *CodeGenTypes::ConvertTypeForMemRecursive(QualType T) { - const llvm::Type *ResultType = ConvertTypeRecursive(T); - if (ResultType->isIntegerTy(1)) - return llvm::IntegerType::get(getLLVMContext(), - (unsigned)Context.getTypeSize(T)); - // FIXME: Should assert that the llvm type and AST type has the same size. - return ResultType; -} - /// ConvertTypeForMem - Convert type T into a llvm::Type. This differs from /// ConvertType in that it is used to convert to the memory representation for /// a type. For example, the scalar representation for _Bool is i1, but the /// memory representation is usually i8 or i32, depending on the target. -const llvm::Type *CodeGenTypes::ConvertTypeForMem(QualType T) { - const llvm::Type *R = ConvertType(T); +const llvm::Type *CodeGenTypes::ConvertTypeForMem(QualType T, bool IsRecursive){ + const llvm::Type *R = ConvertType(T, IsRecursive); // If this is a non-bool type, don't map it. if (!R->isIntegerTy(1)) @@ -284,7 +280,8 @@ const llvm::Type *CodeGenTypes::ConvertNewType(QualType T) { assert(A.getIndexTypeCVRQualifiers() == 0 && "FIXME: We only handle trivial array types so far!"); // int X[] -> [0 x int] - return llvm::ArrayType::get(ConvertTypeForMemRecursive(A.getElementType()), 0); + return llvm::ArrayType::get(ConvertTypeForMemRecursive(A.getElementType()), + 0); } case Type::ConstantArray: { const ConstantArrayType &A = cast(Ty); @@ -299,7 +296,11 @@ const llvm::Type *CodeGenTypes::ConvertNewType(QualType T) { } case Type::FunctionNoProto: case Type::FunctionProto: { - // First, check whether we can build the full function type. + // First, check whether we can build the full function type. If the + // function type depends on an incomplete type (e.g. a struct or enum), we + // cannot lower the function type. Instead, turn it into an Opaque pointer + // and have UpdateCompletedType revisit the function type when/if the opaque + // argument type is defined. if (const TagType *TT = VerifyFuncTypeComplete(&Ty)) { // This function's type depends on an incomplete tag type; make sure // we have an opaque type corresponding to the tag type. @@ -309,17 +310,25 @@ const llvm::Type *CodeGenTypes::ConvertNewType(QualType T) { FunctionTypes.insert(std::make_pair(&Ty, ResultType)); return ResultType; } + // The function type can be built; call the appropriate routines to // build it. - if (const FunctionProtoType *FPT = dyn_cast(&Ty)) - return GetFunctionType(getFunctionInfo( - CanQual::CreateUnsafe(QualType(FPT,0))), - FPT->isVariadic()); - - const FunctionNoProtoType *FNPT = cast(&Ty); - return GetFunctionType(getFunctionInfo( - CanQual::CreateUnsafe(QualType(FNPT,0))), - true); + const CGFunctionInfo *FI; + bool isVariadic; + if (const FunctionProtoType *FPT = dyn_cast(&Ty)) { + FI = &getFunctionInfo( + CanQual::CreateUnsafe(QualType(FPT, 0)), + true /*Recursive*/); + isVariadic = FPT->isVariadic(); + } else { + const FunctionNoProtoType *FNPT = cast(&Ty); + FI = &getFunctionInfo( + CanQual::CreateUnsafe(QualType(FNPT, 0)), + true /*Recursive*/); + isVariadic = true; + } + + return GetFunctionType(*FI, isVariadic, true); } case Type::ObjCObject: diff --git a/lib/CodeGen/CodeGenTypes.h b/lib/CodeGen/CodeGenTypes.h index 674fe8f8b4..bf55fc5369 100644 --- a/lib/CodeGen/CodeGenTypes.h +++ b/lib/CodeGen/CodeGenTypes.h @@ -106,19 +106,22 @@ public: llvm::LLVMContext &getLLVMContext() { return TheModule.getContext(); } /// ConvertType - Convert type T into a llvm::Type. - const llvm::Type *ConvertType(QualType T); + const llvm::Type *ConvertType(QualType T, bool IsRecursive = false); const llvm::Type *ConvertTypeRecursive(QualType T); /// ConvertTypeForMem - Convert type T into a llvm::Type. This differs from /// ConvertType in that it is used to convert to the memory representation for /// a type. For example, the scalar representation for _Bool is i1, but the /// memory representation is usually i8 or i32, depending on the target. - const llvm::Type *ConvertTypeForMem(QualType T); - const llvm::Type *ConvertTypeForMemRecursive(QualType T); + const llvm::Type *ConvertTypeForMem(QualType T, bool IsRecursive = false); + const llvm::Type *ConvertTypeForMemRecursive(QualType T) { + return ConvertTypeForMem(T, true); + } /// GetFunctionType - Get the LLVM function type for \arg Info. const llvm::FunctionType *GetFunctionType(const CGFunctionInfo &Info, - bool IsVariadic); + bool IsVariadic, + bool IsRecursive = false); const llvm::FunctionType *GetFunctionType(GlobalDecl GD); @@ -154,8 +157,11 @@ public: return getFunctionInfo(Ty->getResultType(), Args, Ty->getExtInfo()); } - const CGFunctionInfo &getFunctionInfo(CanQual Ty); - const CGFunctionInfo &getFunctionInfo(CanQual Ty); + + const CGFunctionInfo &getFunctionInfo(CanQual Ty, + bool IsRecursive = false); + const CGFunctionInfo &getFunctionInfo(CanQual Ty, + bool IsRecursive = false); // getFunctionInfo - Get the function info for a member function. const CGFunctionInfo &getFunctionInfo(const CXXRecordDecl *RD, @@ -176,7 +182,8 @@ public: /// \param ArgTys - must all actually be canonical as params const CGFunctionInfo &getFunctionInfo(CanQualType RetTy, const llvm::SmallVectorImpl &ArgTys, - const FunctionType::ExtInfo &Info); + const FunctionType::ExtInfo &Info, + bool IsRecursive = false); /// \brief Compute a new LLVM record layout object for the given record. CGRecordLayout *ComputeRecordLayout(const RecordDecl *D); @@ -189,7 +196,8 @@ public: // These are internal details of CGT that shouldn't be used externally. /// GetExpandedTypes - Expand the type \arg Ty into the LLVM /// argument types it would be passed as on the provided vector \arg /// ArgTys. See ABIArgInfo::Expand. - void GetExpandedTypes(QualType Ty, std::vector &ArgTys); + void GetExpandedTypes(QualType Ty, std::vector &ArgTys, + bool IsRecursive); /// ContainsPointerToDataMember - Return whether the given type contains a /// pointer to a data member. diff --git a/lib/CodeGen/TargetInfo.cpp b/lib/CodeGen/TargetInfo.cpp index 2ba19f1096..c65f203715 100644 --- a/lib/CodeGen/TargetInfo.cpp +++ b/lib/CodeGen/TargetInfo.cpp @@ -830,8 +830,7 @@ X86_64ABIInfo::Class X86_64ABIInfo::merge(Class Accum, Class Field) { return SSE; } -void X86_64ABIInfo::classify(QualType Ty, - uint64_t OffsetBase, +void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo, Class &Hi) const { // FIXME: This code can be simplified by introducing a simple value class for // Class pairs with appropriate constructor methods for the various diff --git a/test/CodeGen/decl.c b/test/CodeGen/decl.c index 7ffb7006b0..dcf120fd88 100644 --- a/test/CodeGen/decl.c +++ b/test/CodeGen/decl.c @@ -89,3 +89,16 @@ struct test7s { int a; int b; } test7[] = { struct test8s { int f0; char f1; } test8g = {}; +// PR7519 + +struct S { + void (*x) (struct S *); +}; + +extern struct S *global_dc; +void cp_diagnostic_starter(struct S *); + +void init_error(void) { + global_dc->x = cp_diagnostic_starter; +} + diff --git a/test/CodeGenCXX/x86_64-arguments.cpp b/test/CodeGenCXX/x86_64-arguments.cpp index 8a21fddec4..df0c78ad94 100644 --- a/test/CodeGenCXX/x86_64-arguments.cpp +++ b/test/CodeGenCXX/x86_64-arguments.cpp @@ -30,3 +30,18 @@ typedef int s4::* s4_mdp; typedef int (s4::*s4_mfp)(); s4_mdp f4_0(s4_mdp a) { return a; } s4_mfp f4_1(s4_mfp a) { return a; } + + +namespace PR7523 { +struct StringRef { + char *a; +}; + +void AddKeyword(StringRef, int x); + +void foo() { + // CHECK: define void @_ZN6PR75233fooEv() + // CHECK: call void @_ZN6PR752310AddKeywordENS_9StringRefEi(i8* {{.*}}, i32 4) + AddKeyword(StringRef(), 4); +} +} \ No newline at end of file -- 2.40.0