[cfe-dev] Implementation of MSRecordLayoutBuilder.

r4start r4start at gmail.com
Mon Sep 19 03:30:10 PDT 2011 

On 15/09/2011 19:35, r4start wrote:
> 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.
Hi Eli,
can you review my patch?

  - Dmitry.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110919/8b0a38c1/attachment.html>

More information about the cfe-dev mailing list