[PATCH] Make thunk this/return adjustment ABI-specific. Also, fix the return adjustment when using -cxx-abi microsoft

Timur Iskhodzhanov timurrrr at google.com
Sun Oct 27 11:17:39 PDT 2013


  PTAL


================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:977
@@ +976,3 @@
+
+  llvm::Type *Int8PtrTy = CGF.Int8PtrTy;
+  llvm::Value *V = CGF.Builder.CreateBitCast(This, Int8PtrTy);
----------------
Reid Kleckner wrote:
> Any need for this local used once?
Good point, fixed.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:980-981
@@ +979,4 @@
+
+  assert(TA.VCallOffsetOffset == 0 &&
+         "VtorDisp adjustment is not supported yet");
+
----------------
Reid Kleckner wrote:
> I'd prefer CGF.ErrorUnsupported instead.  That way we get a clang diagnostic in every build config and it can be easily scraped from logs.
AST/VFTableBuilder doesn't set nonzero VCallOffsetOffset yet, so this really shouldn't be hit.
I'll replace the assert with a proper codegen in the following patch, so not sure it's worth changing.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1000
@@ +999,3 @@
+
+  llvm::Type *Int8PtrTy = CGF.Int8PtrTy;
+  llvm::Value *V = CGF.Builder.CreateBitCast(Ret, Int8PtrTy);
----------------
Reid Kleckner wrote:
> No need for a local used once?
Ditto.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1005-1010
@@ +1004,8 @@
+    assert(RA.Virtual.VBIndex > 0);
+    llvm::Value *VBPtr,
+        *VBPtrOffset =
+            llvm::ConstantInt::get(CGM.IntTy, RA.Virtual.VBPtrOffset),
+        *VBOffset = llvm::ConstantInt::get(CGM.IntTy, 4 * RA.Virtual.VBIndex),
+        *VBaseOffset =
+            GetVBaseOffsetFromVBPtr(CGF, V, VBOffset, VBPtrOffset, &VBPtr);
+    V = CGF.Builder.CreateInBoundsGEP(VBPtr, VBaseOffset);
----------------
Reid Kleckner wrote:
> Declaring multiple variables like this isn't really common style.  VBPtrOffset and VBOffset are constant so it doesn't matter, but VBPtr and VBaseOffset are not.
OK, I've added a handy override (will need it for this adjustment too) so this is much simpler now. Can you please take a look?

================
Comment at: test/CodeGenCXX/microsoft-abi-vtables-multiple-nonvirtual-inheritance.cpp:275
@@ -274,3 +274,3 @@
   // THIS-THUNKS-Test1-NEXT: 0 | void this_adjustment::Test1::g()
-  // THIS-THUNKS-Test1-NEXT:     [this adjustment: -4 non-virtual]
+  // THIS-THUNKS-Test1-NEXT:     this adjustment: -4 non-virtual
 
----------------
FTR, I've decided to bring back []


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



More information about the cfe-commits mailing list