[cfe-dev] Implementation of MSRecordLayoutBuilder.
r4start
r4start at gmail.com
Thu Sep 15 08:35:01 PDT 2011
On 15/09/2011 16:17, Don Williamson wrote:
> That sounds great!
>
> ------------------------------------------------------------------------
> *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:35 PM
> *Subject:* Re: [cfe-dev] Implementation of MSRecordLayoutBuilder.
>
> 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> <mailto:r4start at gmail.com>
>> *To:* Don Williamson <don.williamson at yahoo.com>
>> <mailto:don.williamson at yahoo.com>
>> *Cc:* cfe-dev at cs.uiuc.edu <mailto: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.
>
>
Now it seems working fine.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110915/33cb48d9/attachment.html>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: microsoft-layout-support.patch
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110915/33cb48d9/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ms_class_layout.cpp
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110915/33cb48d9/attachment-0001.ksh>
More information about the cfe-dev
mailing list