[cfe-dev] Implementation of MSRecordLayoutBuilder.

[r4start]

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>
> *To:* John McCall <rjmccall at apple.com>
> *Cc:* 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110915/24dccc59/attachment.html>