[PATCH] Microsoft C Record Layout

Warren Hunt whunt at google.com
Mon Jun 24 16:51:43 PDT 2013



================
Comment at: lib/AST/RecordLayoutBuilder.cpp:2529
@@ +2528,3 @@
+        getTargetInfo().getCXXABI().isMicrosoft() &&
+        !D->isMsStruct(*this))
+      NewEntry = BuildMicrosoftASTRecordLayout(D);
----------------
Reid Kleckner wrote:
> I thought we discussed doing bitfields in this patch, which is what MsStruct is all about, right?  Do the tests pass if you flip the logic here?
This is necessary until I patch the back-end CGRecordLayout code.  It reads that field and does some things that ICE the compiler if I don't emulate the behavior of the _current_ implementation of bitfields on the front end.

================
Comment at: lib/AST/RecordLayoutBuilder.cpp:2528
@@ +2527,3 @@
+    if (getTargetInfo().getPointerWidth(0) == 32 &&
+        getTargetInfo().getCXXABI().isMicrosoft() &&
+        !D->isMsStruct(*this))
----------------
Reid Kleckner wrote:
> This doesn't really make sense, since we never set -cxx-abi microsoft for plain C tests.  This should probably be checking for Triple::Win32.  That should exclude the Mingw and Cygwin ABIs.
> 
> 
Interesting.  I'm setting the flag on my C tests and using it to branch to my code.  I didn't even think that it's a C++ flag and really shouldn't be relevant in the C world.  I agree that triple is a better idea, I'll go ahead and change it.

================
Comment at: lib/AST/RecordLayoutBuilder.cpp:2527
@@ -2526,10 +2526,3 @@
   } else {
-    RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0);
-    Builder.Layout(D);
-
-    NewEntry =
-      new (*this) ASTRecordLayout(*this, Builder.getSize(), 
-                                  Builder.Alignment,
-                                  Builder.getSize(),
-                                  Builder.FieldOffsets.data(),
-                                  Builder.FieldOffsets.size());
+    if (getTargetInfo().getPointerWidth(0) == 32 &&
+        getTargetInfo().getCXXABI().isMicrosoft() &&
----------------
Reid Kleckner wrote:
> I'd probably go with getTargetInfo().getTriple().getArch() == Triple::x86.
Done.


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



More information about the cfe-commits mailing list