[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