[cfe-dev] Implementation of MSRecordLayoutBuilder.
r4start
r4start at gmail.com
Thu Sep 15 04:35:10 PDT 2011
On 15/09/2011 15:14, Don Williamson wrote:
> Sure, I started a previous thread about this:
>
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016941.html
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016942.html
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016944.html
> http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-September/016970.html
>
> It boils down to two problems I've had:
>
> * If there is a double anywhere in a struct, MSVC pads the vftbl pointer.
> * The IsPODForThePurposeOfLayout check is invalid for MS targets and
> so you get a different layout depending upon whether your type has a
> struct or not.
>
> I suggested a simple patch in my last post that fixes these. My test
> cases so far are here:
>
> https://bitbucket.org/dwilliamson/clreflect/src/tip/src/clReflectTest/TestOffsets.cpp
>
> my patch works for all of them but unfortunately I haven't had time to
> sync to the lastest build of clang so I'm not in a position to submit
> a patch.
>
> It would be nice if we could figure something out that solves the
> biggest number of cases :)
>
> Cheers,
> - Don
>
> ------------------------------------------------------------------------
> *From:* r4start <r4start at gmail.com>
> *To:* Don Williamson <don.williamson at yahoo.com>
> *Cc:* cfe-dev at cs.uiuc.edu
> *Sent:* Thursday, September 15, 2011 12:06 PM
> *Subject:* Re: [cfe-dev] Implementation of MSRecordLayoutBuilder.
>
> On 15/09/2011 14:52, Don Williamson wrote:
>> Hi Dmitry,
>>
>> I'm really interested in the original test cases you have that failed
>> with the original implementation of the record layout builder - could
>> you share them?
>>
>> How does your current patch work with the following test:
>>
>> #pragma pack(push, 8)
>> class B {
>> public:
>> virtual void b(){}
>> int a;
>> double b;
>> };
>> #pragma pack(pop)
>>
>> Have you also tried test cases with and without constructors?
>>
>> Thanks,
>> - Don
>>
>> ------------------------------------------------------------------------
>> *From:* r4start <r4start at gmail.com> <mailto:r4start at gmail.com>
>> *To:* John McCall <rjmccall at apple.com> <mailto:rjmccall at apple.com>
>> *Cc:* cfe-dev at cs.uiuc.edu <mailto:cfe-dev at cs.uiuc.edu>
>> *Sent:* Thursday, September 15, 2011 11:20 AM
>> *Subject:* Re: [cfe-dev] Implementation of MSRecordLayoutBuilder.
>>
>> On 15/09/2011 00:39, r4start wrote:
>> > On 15/09/2011 00:36, John McCall wrote:
>> >> On Sep 14, 2011, at 1:33 PM, r4start wrote:
>> >>
>> >>> On 15/09/2011 00:24, John McCall wrote:
>> >>>> On Sep 14, 2011, at 1:23 PM, r4start wrote:
>> >>>>> On 14/09/2011 21:53, John McCall wrote:
>> >>>>>> On Sep 14, 2011, at 4:05 AM, r4start wrote:
>> >>>>>>
>> >>>>>>> On 14/09/2011 00:42, r4start wrote:
>> >>>>>>>> On 14/09/2011 00:37, Eli Friedman wrote:
>> >>>>>>>>> On Mon, Sep 12, 2011 at 11:45 PM, r4start<r4start at gmail.com
>> <mailto:r4start at gmail.com>> wrote:
>> >>>>>>>>>> On 12/09/2011 21:57, r4start wrote:
>> >>>>>>>>>>> On 12/09/2011 21:37, Eli Friedman wrote:
>> >>>>>>>>>>>> On Mon, Sep 12, 2011 at 8:56 AM,
>> r4start<r4start at gmail.com <mailto:r4start at gmail.com>> wrote:
>> >>>>>>>>>>>>> Hi,
>> >>>>>>>>>>>>> I have some prototype code for MSRecordLayoutBuilder. I
>> test
>> >>>>>>>>>>>>> this code
>> >>>>>>>>>>>>> with MSVS 2010 and \Zp8 compiler option. On simple
>> examples it
>> >>>>>>>>>>>>> seems to
>> >>>>>>>>>>>>> work properly.
>> >>>>>>>>>>>>>
>> >>>>>>>>>>>>> Is it interesting to the clang project?
>> >>>>>>>>>>>> We certainly want to fix any cases where clang does not
>> do struct
>> >>>>>>>>>>>> layout consistently with MSVC on Windows. It's hard to
>> comment on
>> >>>>>>>>>>>> your patch without seeing it.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> -Eli
>> >>>>>>>>>>> Now I'm not at work. When I come to work I'll send a patch.
>> >>>>>>>>>>>
>> >>>>>>>>>>> - Dmitry
>> >>>>>>>>>> Here is patch for MSRecordLayoutBuilder.
>> >>>>>>>>> I think it would be better to integrate the checks into the
>> main
>> >>>>>>>>> RecordLayoutBuilder class rather than subclassing it.
>> >>>>>>>>>
>> >>>>>>>>> Please include tests in your patch. (See
>> test/CodeGen/ms_struct.c for
>> >>>>>>>>> an example.)
>> >>>>>>>>>
>> >>>>>>>>> I would like someone more familiar with MSVC to review the
>> actual
>> >>>>>>>>> logic here (ping me if nobody does within a few days,
>> though, and I'll
>> >>>>>>>>> try to review anyway).
>> >>>>>>>>>
>> >>>>>>>>> -Eli
>> >>>>>>>> Ok, thanks for hint.
>> >>>>>>>>
>> >>>>>>>> -Dmitry.
>> >>>>>>> I have a problem when writing a test. Clang doesn't implement
>> Microsoft
>> >>>>>>> ABI and he crashes at code generation stage.
>> >>>>>>> Clang have dump-record-layouts option but he prints AST
>> layouts and
>> >>>>>>> CodeGen layouts.
>> >>>>>>>
>> >>>>>>> Is there a way to save only AST record layout to file and
>> then check
>> >>>>>>> or maybe my patch must include CGRecordLayoutBuilder for
>> Microsoft?
>> >>>>>> It would be easy enough to add a new testing switch if you'd
>> like to get this in.
>> >>>>> Can you give me some hint how can I do this?
>> >>>> Follow the code for -dump-record-layouts.
>> >>> You propose to duplicate the output to a file and then run
>> FileCheck to check output, am I right?
>> >> It would probably be better to run FileCheck directly on the
>> output, but
>> >> I think you get the general idea.
>> >>
>> >> Whenever you get CGRecordLayoutBuilder working adequately, you can rip
>> >> out the new option and just use -dump-record-layouts.
>> >>
>> >> John.
>> > Ok, thanks. Tomorrow I'll do it.
>> >
>> Here is a test and patch.
>>
>> - Dmitry.
>>
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu <mailto:cfe-dev at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>>
>>
> Hi,
> what I get from MSVS output :
>
> class B size(24):
> 1> +---
> 1> 0 | {vfptr}
> 1> 8 | a
> 1> | <alignment member> (size=4)
> 1> 16 | c
> 1> +---
>
> And this I get from clang with my patch:
> 0 | class B
> 0 | (B vtable pointer)
> 4 | int a
> 8 | double c
> sizeof=16, dsize=16, align=8
> nvsize=16, nvalign=8
>
> I'll try to fix this.
>
> What do you mean when you say "Have you also tried test cases with and
> without constructors?", can you give me some examples and I check them.
>
> - Dmitry.
>
>
Thanks for examples. I'll add your patch to my code.
I think it will be great if we will have full support for Microsoft layout.
- Dmitry.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110915/e0c93416/attachment.html>
More information about the cfe-dev
mailing list