This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: Recent go changes broke alpha bootstrap
- From: Ian Taylor <iant at golang dot org>
- To: Dominik Vogt <vogt at linux dot vnet dot ibm dot com>
- Cc: GCC Development <gcc at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Uros Bizjak <ubizjak at gmail dot com>
- Date: Mon, 3 Nov 2014 16:00:45 -0800
- Subject: Re: Recent go changes broke alpha bootstrap
- Authentication-results: sourceware.org; auth=none
- References: <CAFULd4byXqjbnt-yy_pDSf0BvgNjchj-pyypMgmOXaw_vZmTng at mail dot gmail dot com> <CAFULd4bWyOORxtZM-TF4Z4nr-1EYwL34gdVLxxeaPC8jOx16jA at mail dot gmail dot com> <20141030124621 dot GA18230 at linux dot vnet dot ibm dot com> <CAOyqgcUF7puBgZKX_9jJCofKZ1Aa4dL6co1suGm3u_Y28sV+mg at mail dot gmail dot com> <20141103170240 dot GA11956 at linux dot vnet dot ibm dot com>
On Mon, Nov 3, 2014 at 9:02 AM, Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
> On Thu, Oct 30, 2014 at 08:05:14AM -0700, Ian Taylor wrote:
>> On Thu, Oct 30, 2014 at 5:46 AM, Dominik Vogt <vogt@linux.vnet.ibm.com> wrote:
>> > I'm not quite sure about the best approach. The attempt to
>> > represent C unions in the "right" way is ultimately futile as Go
>> > does not have the types necessary for it. For example, the
>> > handling of anonymous bit fields will never be right as it's
>> > undefinied. On the other hand I could fix the issue at hand by
>> > changing the way anonymous unions are represented in Go.
>> >
>> > Example:
>> >
>> > struct { int8_t x; union { int16_t y; int 32_t z; }; };
>> >
>> > Was represented (before the patch) as
>> >
>> > struct { X byte; int16 Y; }
>> >
>> > which had size 4, alignment 2 and y at offset 2 but should had
>> > have size 8, alignment 4 and y at offset 4. With the current
>> > patch the Go layout is
>> >
>> > struct { X byte; artificial_name struct { y [2]byte; align [0]int32; } }
>> >
>> > with the proper size, alignment and offset, but y is addressed as
>> > ".artificial_name.y" insted of just ".y", and y is a byte array
>> > and not an int16.
>> >
>> > I could remove the "artificial_name struct" and add padding before
>> > and after y instead:
>> >
>> > struct { X byte; pad_0 [3]byte; Y int16; pad_1 [2]byte; align [0]int32; }
>> >
>> > What do you think?
>>
>> Sounds good to me. Basically the fields of the anonymous union should
>> be promoted to become fields of the struct. We can't do it in
>> general, but we can do it for the first field. That addresses the
>> actual uses of anonymous unions.
>
> The attached patch fixes this, at least if the first element of the
> union is not a bitfield.
>
> Bitfields can really not be represented properly in Go (think about
> constructs like "struct { int : 1; int bf : 1; }"), I'd rather not
> try to represent them in a predictable way. The patched code may
> or may not give them a name, and reserves the proper amount of
> padding where required in structs. If a union begins with an
> anonymous bitfield (which makes no sense), that is ignored. If a
> union begins with a named bitfield (possibly after unnamed ones),
> this may or may not be used as the (sole) element of the Go
> struct.
Thanks. I committed your patch.
Ian