[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