[PATCH] Abstract out virtual calls and virtual function prologue code generation

Timur Iskhodzhanov timurrrr at google.com
Fri Aug 2 02:34:50 PDT 2013


  John,
  Any comments on this?


================
Comment at: lib/CodeGen/CGCXXABI.h:208
@@ +207,3 @@
+  virtual llvm::Value*
+  adjustThisArgumentInPrologue(CodeGenFunction &CGF, GlobalDecl GD,
+                               llvm::Value *This) {
----------------
Reid Kleckner wrote:
> Can you work "virtual" into the method name somehow?  Only the comment says anything about virtual methods.  Also, once we're in the method, 'this' is a parameter, not an argument.
Good points, done.

================
Comment at: lib/CodeGen/CGExprCXX.cpp:316
@@ +315,3 @@
+  if (MD->isVirtual())
+    This = CGM.getCXXABI().adjustThisArgumentForVirtualCall(*this, MD, This);
+
----------------
Reid Kleckner wrote:
> It looks like this is the only caller of this ABI hook.  I guess it's better to expose two entry points than it is to take a boolean into BuildVirtualCall.
Wait, the argument adjustment is also needed for devirtualized calls, so this can't be done only in BuildVirtualCall, right?

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:124
@@ +123,3 @@
+
+  virtual llvm::Value*
+  adjustThisArgumentForVirtualCall(CodeGenFunction &CGF, GlobalDecl GD,
----------------
Reid Kleckner wrote:
> Doesn't really matter, but clang-format will spell this "Value *" here and elsewhere.
Fixed

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:114
@@ -113,1 +113,3 @@
 
+  const CXXRecordDecl* getThisArgumentTypeForMethod(const CXXMethodDecl *MD) {
+    MD = MD->getCanonicalDecl();
----------------
Reid Kleckner wrote:
> nit: star on the right
Fixed


http://llvm-reviews.chandlerc.com/D1260



More information about the cfe-commits mailing list