[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/CodeGe
Tilmann Scheller
tilmann.scheller at googlemail.com
Wed Mar 2 13:42:12 PST 2011
Reverted in r126886.
Will try to come up with a better patch.
Regards,
Tilmann
On Wed, Mar 2, 2011 at 10:01 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110302/b145de97/attachment.html>
More information about the cfe-commits
mailing list