[cfe-commits] r126863 - in /cfe/trunk: include/clang/AST/Type.h include/clang/Basic/TargetInfo.h lib/AST/DumpXML.cpp lib/AST/MicrosoftMangle.cpp lib/AST/Type.cpp lib/CodeGen/CGBlocks.cpp lib/CodeGen/CGCall.cpp lib/CodeGen/CGDeclCXX.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprCXX.cpp lib/CodeGen/CGObjC.cpp lib/CodeGen/CGVTables.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenFunction.h lib/CodeGen/CodeGenTypes.h lib/CodeGen/TargetInfo.cpp
Douglas Gregor
dgregor at apple.com
Wed Mar 2 13:01:24 PST 2011
On Mar 2, 2011, at 11:36 AM, Tilmann Scheller wrote:
> Author: tilmann
> Date: Wed Mar 2 13:36:23 2011
> New Revision: 126863
>
> URL: http://llvm.org/viewvc/llvm-project?rev=126863&view=rev
> Log:
> Add CC_Win64ThisCall and set it in the necessary places.
Some comments below.
> Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=126863&r1=126862&r2=126863&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Mar 2 13:36:23 2011
> @@ -36,6 +36,7 @@
> case CC_X86StdCall: return llvm::CallingConv::X86_StdCall;
> case CC_X86FastCall: return llvm::CallingConv::X86_FastCall;
> case CC_X86ThisCall: return llvm::CallingConv::X86_ThisCall;
> + case CC_Win64ThisCall: return llvm::CallingConv::Win64_ThisCall;
> // TODO: add support for CC_X86Pascal to llvm
> }
> }
> @@ -75,19 +76,23 @@
> static const CGFunctionInfo &getFunctionInfo(CodeGenTypes &CGT,
> llvm::SmallVectorImpl<CanQualType> &ArgTys,
> CanQual<FunctionProtoType> FTP,
> + CallingConv CC,
> 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(), IsRecursive);
> +
> + return CGT.getFunctionInfo(ResTy, ArgTys,
> + FTP->getExtInfo().withCallingConv(CC),
> + IsRecursive);
> }
>
> const CGFunctionInfo &
> CodeGenTypes::getFunctionInfo(CanQual<FunctionProtoType> FTP,
> bool IsRecursive) {
> llvm::SmallVector<CanQualType, 16> ArgTys;
> - return ::getFunctionInfo(*this, ArgTys, FTP, IsRecursive);
> + return ::getFunctionInfo(*this, ArgTys, FTP, CC_Default, IsRecursive);
> }
>
> static CallingConv getCallingConventionForDecl(const Decl *D) {
> @@ -114,8 +119,10 @@
> // Add the 'this' pointer.
> ArgTys.push_back(GetThisType(Context, RD));
>
> + CallingConv CC = Context.Target.isWin64() ? CC_Win64ThisCall : CC_Default;
> +
This kind of thing makes me very nervous. Why isn't the calling convention already part of the function type when we get here? And, if there is a calling convention specified on the function type (e.g., through an attribute), won't we end up ignoring it?
> return ::getFunctionInfo(*this, ArgTys,
> - FTP->getCanonicalTypeUnqualified().getAs<FunctionProtoType>());
> + FTP->getCanonicalTypeUnqualified().getAs<FunctionProtoType>(), CC);
> }
>
> const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const CXXMethodDecl *MD) {
> @@ -128,7 +135,9 @@
> if (MD->isInstance())
> ArgTys.push_back(GetThisType(Context, MD->getParent()));
>
> - return ::getFunctionInfo(*this, ArgTys, GetFormalType(MD));
> + CallingConv CC = Context.Target.isWin64() ? CC_Win64ThisCall : CC_Default;
> +
> + return ::getFunctionInfo(*this, ArgTys, GetFormalType(MD), CC);
> }
Same questions/concerns as above.
Also, do static member functions really use CC_Win64ThisCall, even when there is no "this"?
> const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const CXXConstructorDecl *D,
> @@ -145,7 +154,9 @@
> for (unsigned i = 0, e = FTP->getNumArgs(); i != e; ++i)
> ArgTys.push_back(FTP->getArgType(i));
>
> - return getFunctionInfo(ResTy, ArgTys, FTP->getExtInfo());
> + CallingConv CC = Context.Target.isWin64() ? CC_Win64ThisCall : CC_Default;
> +
> + return getFunctionInfo(ResTy, ArgTys, FTP->getExtInfo().withCallingConv(CC));
> }
Again :)
> const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const CXXDestructorDecl *D,
> @@ -159,7 +170,9 @@
> CanQual<FunctionProtoType> FTP = GetFormalType(D);
> assert(FTP->getNumArgs() == 0 && "dtor with formal parameters");
>
> - return getFunctionInfo(ResTy, ArgTys, FTP->getExtInfo());
> + CallingConv CC = Context.Target.isWin64() ? CC_Win64ThisCall : CC_Default;
> +
> + return getFunctionInfo(ResTy, ArgTys, FTP->getExtInfo().withCallingConv(CC));
> }
Again :)
> const CGFunctionInfo &CodeGenTypes::getFunctionInfo(const FunctionDecl *FD) {
> @@ -250,8 +263,8 @@
> return *FI;
>
> // Construct the function info.
> - FI = new CGFunctionInfo(CC, Info.getNoReturn(), Info.getRegParm(), ResTy,
> - ArgTys.data(), ArgTys.size());
> + FI = new CGFunctionInfo(CC, Info.getNoReturn(), Info.getRegParm(),
> + ResTy, ArgTys.data(), ArgTys.size());
> FunctionInfos.InsertNode(FI, InsertPos);
>
> // Compute ABI information.
>
> Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=126863&r1=126862&r2=126863&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Wed Mar 2 13:36:23 2011
> @@ -261,7 +261,7 @@
> const VarDecl *D,
> llvm::GlobalVariable *Addr) {
> StartFunction(GlobalDecl(), getContext().VoidTy, Fn, FunctionArgList(),
> - SourceLocation());
> + SourceLocation(), CC_Default);
>
> // Use guarded initialization if the global variable is weak due to
> // being a class template's static data member.
> @@ -278,7 +278,7 @@
> llvm::Constant **Decls,
> unsigned NumDecls) {
> StartFunction(GlobalDecl(), getContext().VoidTy, Fn, FunctionArgList(),
> - SourceLocation());
> + SourceLocation(), CC_Default);
>
> for (unsigned i = 0; i != NumDecls; ++i)
> if (Decls[i])
> @@ -290,8 +290,10 @@
> void CodeGenFunction::GenerateCXXGlobalDtorFunc(llvm::Function *Fn,
> const std::vector<std::pair<llvm::WeakVH, llvm::Constant*> >
> &DtorsAndObjects) {
> + const bool IsWin64 = getContext().Target.isWin64();
> +
> StartFunction(GlobalDecl(), getContext().VoidTy, Fn, FunctionArgList(),
> - SourceLocation());
> + SourceLocation(), IsWin64 ? CC_Win64ThisCall : CC_Default);
Again :)
If we're going to keep this operation of "adjust actual calling convention for a member function" within CodeGen (as opposed to, say, moving it into Sema somehow), please abstract it into a function or make it part of the ABI abstraction layer.
> [snipped a bunch more cases]
> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=126863&r1=126862&r2=126863&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Wed Mar 2 13:36:23 2011
> @@ -942,6 +942,26 @@
>
> return false;
> }
> +
> + void SetTargetAttributes(const Decl *D,
> + llvm::GlobalValue *GV,
> + CodeGen::CodeGenModule &M) const {
> + if (M.getContext().Target.isWin64()) {
> + if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
> + const CXXMethodDecl *M = dyn_cast<CXXMethodDecl>(FD);
> +
> + if ((M && M->isInstance())
> + || dyn_cast<CXXConstructorDecl>(FD)
> + || dyn_cast<CXXDestructorDecl>(FD)) {
> + // When using the MSVC Win64 ABI, methods/constructors/destructors
> + // use the Win64 thiscall calling convention.
> + llvm::Function *F = cast<llvm::Function>(GV);
> + F->setCallingConv(llvm::CallingConv::Win64_ThisCall);
> + }
> + }
> + }
> + }
> +
> };
This needs to check whether the calling convention has been overridden.
I feel like there's a much cleaner solution than what is in this patch. The number of places where it's checking for Win64-specific behavior is unnerving, so at the very least we need to abstract that out into some more general function (e.g., getDefaultCallingConventionFor(Decl*)). Given that, and the buildbot breakage, it might be best to revert this commit and see how cleanly we can introduce this behavior.
- Doug
More information about the cfe-commits
mailing list