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 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
