[PATCH] [-cxx-abi microsoft] Emit thunks for pointers to virtual member functions
Timur Iskhodzhanov
timurrrr at google.com
Thu Nov 14 04:53:24 PST 2013
Looks good with nits/suggestions but I'd like Reid to approve too.
================
Comment at: lib/CodeGen/CGVTables.cpp:243
@@ +242,3 @@
+ const CGFunctionInfo &FnInfo) {
+ assert(!CurGD.getDecl() && "CurGD was already set!");
+ CurGD = GD;
----------------
How about "Please use a new CGF for this thunk" instead?
I think such a string would make it clearer what one is doing wrong if this fires.
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1005
@@ +1004,3 @@
+ CGF.EmitCallAndReturnForThunk(MD, Callee, This, 0);
+
+ CGF.FinishFunction();
----------------
Just curious: there's an AutoreleaseResult assignment before FinishFunction when emitting standard thunks.
Have you checked whether we need that for vmemfunptr thunks?
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1434
@@ +1433,3 @@
+ FirstField = llvm::Constant::getNullValue(CGM.VoidPtrTy);
+ } else if (CGM.getMicrosoftVTableContext()
+ .getMethodVFTableLocation(MD)
----------------
how about
} else {
... &ML = CGM.getMicrosoftVTableContext().getMethodVFTableLocation(MD);
if (ML.VBase) {
...
} else {
...
}
}
?
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1446
@@ +1445,3 @@
+ bool Is64Bit = getContext().getTargetInfo().getPointerWidth(0) == 64;
+ int OffsetInVFTable = ML.Index * (Is64Bit ? 8 : 4);
+ llvm::raw_svector_ostream Out(ThunkName);
----------------
Earlier comment by Reid says:
IMO the right way to write this is something like:
ML.Index * getContext().getTypeSizeInChars(getContext().VoidPtrTy)
================
Comment at: lib/CodeGen/CodeGenFunction.h:1159
@@ +1158,3 @@
+ void EmitCallAndReturnForThunk(GlobalDecl GD, llvm::Value *Callee,
+ llvm::Value *AdjustedThisPtr,
+ const ThunkInfo *Thunk);
----------------
Is there any strong reason why "this" adjustment can't be performed by this function?
http://llvm-reviews.chandlerc.com/D2104
More information about the cfe-commits
mailing list