I think we should have ABI-specific thunks, something like MSThunkInfo, 
MSThisAdjustment, MSReturnAdjustment.  With the change you've written here, 
CGVTables will no longer access fields of ThunkInfo, instead it will pass them 
through to the ABI code.

  However, splitting apart the uses of ThunkInfoVectorTy + ThunkMapTy seems 
hard.  What do you think?  Is it worth spending a few hours on it to see if it 
can be done?

  I probably wouldn't go so far as to add support for cast<> to these, because 
it would require space in ThunkInfo, and we allocate these things in arrays.


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

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

================
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);
----------------
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.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:984-987
@@ +983,6 @@
+  if (TA.NonVirtual) {
+    // Non-virtual adjustment might result in a pointer outside the allocated
+    // object, e.g. if the final overrider class is laid out after the virtual
+    // base that declares a method in the most derived class.
+    V = CGF.Builder.CreateConstGEP1_32(V, TA.NonVirtual);
+  }
----------------
Yup.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:980-981
@@ +979,4 @@
+
+  assert(TA.VCallOffsetOffset == 0 &&
+         "VtorDisp adjustment is not supported yet");
+
----------------
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.


http://llvm-reviews.chandlerc.com/D2026
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to