From: John McCall Date: Wed, 24 Feb 2010 07:14:12 +0000 (+0000) Subject: Canonicalize parameter and return types before computing ABI info. Eliminates X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0b0ef0a70b8010c66fad2603e4423ef1c1dc7015;p=clang Canonicalize parameter and return types before computing ABI info. Eliminates a common source of oddities and, in theory, removes some redundant ABI computations. Also fixes a miscompile I introduced yesterday by refactoring some code and causing a slightly different code path to be taken that didn't perform *parameter* type canonicalization, just normal type canonicalization; this in turn caused a bit of ABI code to misfire because it was looking for 'double' or 'float' but received 'const float'. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@97030 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp index 1cd899318a..3f10612832 100644 --- a/lib/CodeGen/CGCall.cpp +++ b/lib/CodeGen/CGCall.cpp @@ -41,21 +41,54 @@ static unsigned ClangCallConvToLLVMCallConv(CallingConv CC) { } } -const -CGFunctionInfo &CodeGenTypes::getFunctionInfo(const FunctionNoProtoType *FTNP) { - return getFunctionInfo(FTNP->getResultType(), +/// Derives the 'this' type for codegen purposes, i.e. ignoring method +/// qualification. +/// FIXME: address space qualification? +static QualType GetThisType(ASTContext &Context, const CXXRecordDecl *RD) { + return Context.getPointerType(Context.getTagDeclType(RD)); +} + +/// Returns the canonical formal type of the given C++ method. +static const FunctionProtoType *GetFormalType(const CXXMethodDecl *MD) { + return cast(MD->getType()->getCanonicalTypeInternal()); +} + +/// Returns the "extra-canonicalized" return type, which discards +/// qualifiers on the return type. Codegen doesn't care about them, +/// and it makes ABI code a little easier to be able to assume that +/// all parameter and return types are top-level unqualified. +static QualType GetReturnType(QualType RetTy) { + return RetTy->getCanonicalTypeInternal().getUnqualifiedType(); +} + +const CGFunctionInfo & +CodeGenTypes::getFunctionInfo(const FunctionNoProtoType *FTNP) { + assert(FTNP->isCanonicalUnqualified() && "type must be canonical"); + return getFunctionInfo(GetReturnType(FTNP->getResultType()), llvm::SmallVector(), - FTNP->getCallConv(), FTNP->getNoReturnAttr()); + FTNP->getCallConv(), + FTNP->getNoReturnAttr()); } -const -CGFunctionInfo &CodeGenTypes::getFunctionInfo(const FunctionProtoType *FTP) { - llvm::SmallVector ArgTys; +/// \param Args - contains any initial parameters besides those +/// in the formal type +static const CGFunctionInfo &getFunctionInfo(CodeGenTypes &CGT, + llvm::SmallVectorImpl &ArgTys, + const FunctionProtoType *FTP) { + assert(FTP->isCanonicalUnqualified() && "type must be canonical"); // FIXME: Kill copy. for (unsigned i = 0, e = FTP->getNumArgs(); i != e; ++i) ArgTys.push_back(FTP->getArgType(i)); - return getFunctionInfo(FTP->getResultType(), ArgTys, - FTP->getCallConv(), FTP->getNoReturnAttr()); + return CGT.getFunctionInfo(GetReturnType(FTP->getResultType()), + ArgTys, + FTP->getCallConv(), + FTP->getNoReturnAttr()); +} + +const CGFunctionInfo & +CodeGenTypes::getFunctionInfo(const FunctionProtoType *FTP) { + llvm::SmallVector ArgTys; + return ::getFunctionInfo(*this, ArgTys, FTP); } static CallingConv getCallingConventionForDecl(const Decl *D) { @@ -72,30 +105,22 @@ static CallingConv getCallingConventionForDecl(const Decl *D) { const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const CXXRecordDecl *RD, const FunctionProtoType *FTP) { llvm::SmallVector ArgTys; - + // Add the 'this' pointer. - ArgTys.push_back(Context.getPointerType(Context.getTagDeclType(RD))); - - for (unsigned i = 0, e = FTP->getNumArgs(); i != e; ++i) - ArgTys.push_back(FTP->getArgType(i)); - - // FIXME: Set calling convention correctly, it needs to be associated with the - // type somehow. - return getFunctionInfo(FTP->getResultType(), ArgTys, - FTP->getCallConv(), FTP->getNoReturnAttr()); + ArgTys.push_back(GetThisType(Context, RD)); + + return ::getFunctionInfo(*this, ArgTys, + cast(FTP->getCanonicalTypeInternal())); } const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const CXXMethodDecl *MD) { llvm::SmallVector ArgTys; + // Add the 'this' pointer unless this is a static method. if (MD->isInstance()) - ArgTys.push_back(MD->getThisType(Context)); + ArgTys.push_back(GetThisType(Context, MD->getParent())); - const FunctionProtoType *FTP = MD->getType()->getAs(); - for (unsigned i = 0, e = FTP->getNumArgs(); i != e; ++i) - ArgTys.push_back(FTP->getArgType(i)); - return getFunctionInfo(FTP->getResultType(), ArgTys, FTP->getCallConv(), - FTP->getNoReturnAttr()); + return ::getFunctionInfo(*this, ArgTys, GetFormalType(MD)); } const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const CXXConstructorDecl *D, @@ -103,17 +128,13 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const CXXConstructorDecl *D, llvm::SmallVector ArgTys; // Add the 'this' pointer. - ArgTys.push_back(D->getThisType(Context)); + ArgTys.push_back(GetThisType(Context, D->getParent())); // Check if we need to add a VTT parameter (which has type void **). if (Type == Ctor_Base && D->getParent()->getNumVBases() != 0) ArgTys.push_back(Context.getPointerType(Context.VoidPtrTy)); - - const FunctionProtoType *FTP = D->getType()->getAs(); - for (unsigned i = 0, e = FTP->getNumArgs(); i != e; ++i) - ArgTys.push_back(FTP->getArgType(i)); - return getFunctionInfo(FTP->getResultType(), ArgTys, FTP->getCallConv(), - FTP->getNoReturnAttr()); + + return ::getFunctionInfo(*this, ArgTys, GetFormalType(D)); } const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const CXXDestructorDecl *D, @@ -126,12 +147,8 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const CXXDestructorDecl *D, // Check if we need to add a VTT parameter (which has type void **). if (Type == Dtor_Base && D->getParent()->getNumVBases() != 0) ArgTys.push_back(Context.getPointerType(Context.VoidPtrTy)); - - const FunctionProtoType *FTP = D->getType()->getAs(); - for (unsigned i = 0, e = FTP->getNumArgs(); i != e; ++i) - ArgTys.push_back(FTP->getArgType(i)); - return getFunctionInfo(FTP->getResultType(), ArgTys, FTP->getCallConv(), - FTP->getNoReturnAttr()); + + return ::getFunctionInfo(*this, ArgTys, GetFormalType(D)); } const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const FunctionDecl *FD) { @@ -139,19 +156,11 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const FunctionDecl *FD) { if (MD->isInstance()) return getFunctionInfo(MD); - const FunctionType *FTy = FD->getType()->getAs(); - if (const FunctionNoProtoType *FNTP = dyn_cast(FTy)) - return getFunctionInfo(FNTP->getResultType(), - llvm::SmallVector(), - FNTP->getCallConv(), FNTP->getNoReturnAttr()); - - const FunctionProtoType *FPT = cast(FTy); - llvm::SmallVector ArgTys; - // FIXME: Kill copy. - for (unsigned i = 0, e = FPT->getNumArgs(); i != e; ++i) - ArgTys.push_back(FPT->getArgType(i)); - return getFunctionInfo(FPT->getResultType(), ArgTys, - FPT->getCallConv(), FPT->getNoReturnAttr()); + const FunctionType *FTy + = cast(FD->getType()->getCanonicalTypeInternal()); + if (isa(FTy)) + return getFunctionInfo(cast(FTy)); + return getFunctionInfo(cast(FTy)); } const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const ObjCMethodDecl *MD) { @@ -160,9 +169,11 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const ObjCMethodDecl *MD) { ArgTys.push_back(Context.getObjCSelType()); // FIXME: Kill copy? for (ObjCMethodDecl::param_iterator i = MD->param_begin(), - e = MD->param_end(); i != e; ++i) - ArgTys.push_back((*i)->getType()); - return getFunctionInfo(MD->getResultType(), ArgTys, + e = MD->param_end(); i != e; ++i) { + ArgTys.push_back(Context.getCanonicalParamType((*i)->getType())); + } + return getFunctionInfo(GetReturnType(MD->getResultType()), + ArgTys, getCallingConventionForDecl(MD), /*NoReturn*/ false); } @@ -188,8 +199,8 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(QualType ResTy, llvm::SmallVector ArgTys; for (CallArgList::const_iterator i = Args.begin(), e = Args.end(); i != e; ++i) - ArgTys.push_back(i->second); - return getFunctionInfo(ResTy, ArgTys, CC, NoReturn); + ArgTys.push_back(Context.getCanonicalParamType(i->second)); + return getFunctionInfo(GetReturnType(ResTy), ArgTys, CC, NoReturn); } const CGFunctionInfo &CodeGenTypes::getFunctionInfo(QualType ResTy, @@ -200,12 +211,12 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(QualType ResTy, llvm::SmallVector ArgTys; for (FunctionArgList::const_iterator i = Args.begin(), e = Args.end(); i != e; ++i) - ArgTys.push_back(i->second); - return getFunctionInfo(ResTy, ArgTys, CC, NoReturn); + ArgTys.push_back(Context.getCanonicalParamType(i->second)); + return getFunctionInfo(GetReturnType(ResTy), ArgTys, CC, NoReturn); } const CGFunctionInfo &CodeGenTypes::getFunctionInfo(QualType ResTy, - const llvm::SmallVector &ArgTys, + const llvm::SmallVectorImpl &ArgTys, CallingConv CallConv, bool NoReturn) { unsigned CC = ClangCallConvToLLVMCallConv(CallConv); @@ -233,7 +244,7 @@ const CGFunctionInfo &CodeGenTypes::getFunctionInfo(QualType ResTy, CGFunctionInfo::CGFunctionInfo(unsigned _CallingConvention, bool _NoReturn, QualType ResTy, - const llvm::SmallVector &ArgTys) + const llvm::SmallVectorImpl &ArgTys) : CallingConvention(_CallingConvention), EffectiveCallingConvention(_CallingConvention), NoReturn(_NoReturn) diff --git a/lib/CodeGen/CGCall.h b/lib/CodeGen/CGCall.h index 9601e9ae9a..1d1b8eeab7 100644 --- a/lib/CodeGen/CGCall.h +++ b/lib/CodeGen/CGCall.h @@ -82,7 +82,7 @@ namespace CodeGen { CGFunctionInfo(unsigned CallingConvention, bool NoReturn, QualType ResTy, - const llvm::SmallVector &ArgTys); + const llvm::SmallVectorImpl &ArgTys); ~CGFunctionInfo() { delete[] Args; } const_arg_iterator arg_begin() const { return Args + 1; } diff --git a/lib/CodeGen/CodeGenTypes.h b/lib/CodeGen/CodeGenTypes.h index 7ce96f48f4..f2e12caa65 100644 --- a/lib/CodeGen/CodeGenTypes.h +++ b/lib/CodeGen/CodeGenTypes.h @@ -186,11 +186,6 @@ public: /// replace the 'opaque' type we previously made for it if applicable. void UpdateCompletedType(const TagDecl *TD); -private: - const CGFunctionInfo &getFunctionInfo(const FunctionNoProtoType *FTNP); - const CGFunctionInfo &getFunctionInfo(const FunctionProtoType *FTP); - -public: /// getFunctionInfo - Get the function info for the specified function decl. const CGFunctionInfo &getFunctionInfo(GlobalDecl GD); @@ -207,6 +202,8 @@ public: return getFunctionInfo(Ty->getResultType(), Args, Ty->getCallConv(), Ty->getNoReturnAttr()); } + const CGFunctionInfo &getFunctionInfo(const FunctionProtoType *Ty); + const CGFunctionInfo &getFunctionInfo(const FunctionNoProtoType *Ty); // getFunctionInfo - Get the function info for a member function. const CGFunctionInfo &getFunctionInfo(const CXXRecordDecl *RD, @@ -224,7 +221,7 @@ public: CallingConv CC, bool NoReturn); const CGFunctionInfo &getFunctionInfo(QualType RetTy, - const llvm::SmallVector &ArgTys, + const llvm::SmallVectorImpl &ArgTys, CallingConv CC, bool NoReturn); diff --git a/lib/CodeGen/TargetInfo.cpp b/lib/CodeGen/TargetInfo.cpp index f1cbc56dda..f4ec914a4e 100644 --- a/lib/CodeGen/TargetInfo.cpp +++ b/lib/CodeGen/TargetInfo.cpp @@ -972,12 +972,11 @@ ABIArgInfo X86_64ABIInfo::getCoerceResult(QualType Ty, return (Ty->isPromotableIntegerType() ? ABIArgInfo::getExtend() : ABIArgInfo::getDirect()); } else if (CoerceTo == llvm::Type::getDoubleTy(CoerceTo->getContext())) { - // FIXME: It would probably be better to make CGFunctionInfo only map using - // canonical types than to canonize here. - QualType CTy = Context.getCanonicalType(Ty); + assert(Ty.isCanonical() && "should always have a canonical type here"); + assert(!Ty.hasQualifiers() && "should never have a qualified type here"); // Float and double end up in a single SSE reg. - if (CTy == Context.FloatTy || CTy == Context.DoubleTy) + if (Ty == Context.FloatTy || Ty == Context.DoubleTy) return ABIArgInfo::getDirect(); } diff --git a/test/CodeGen/functions.c b/test/CodeGen/functions.c index cb9a4ef81f..d9c87d53ed 100644 --- a/test/CodeGen/functions.c +++ b/test/CodeGen/functions.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 %s -emit-llvm -o %t +// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s int g(); @@ -38,3 +38,12 @@ struct foo { int X, Y, Z; } f3() { // PR4423 - This shouldn't crash in codegen void f4() {} void f5() { f4(42); } + +// Qualifiers on parameter types shouldn't make a difference. +static void f6(const float f, const float g) { +} +void f7(float f, float g) { + f6(f, g); +// CHECK: define void @f7(float{{.*}}, float{{.*}}) +// CHECK: call void @f6(float{{.*}}, float{{.*}}) +} diff --git a/test/CodeGenObjC/messages-2.m b/test/CodeGenObjC/messages-2.m index 2a6e3dcbbd..05e30ab131 100644 --- a/test/CodeGenObjC/messages-2.m +++ b/test/CodeGenObjC/messages-2.m @@ -136,4 +136,7 @@ typedef struct { x.height *= 2; return x; } +-(const float) returnAConstFloat { + return 5; +} @end