This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Recent go changes broke alpha bootstrap
- From: Uros Bizjak <ubizjak at gmail dot com>
- To: Ian Taylor <iant at golang dot org>
- Cc: Dominik Vogt <vogt at linux dot vnet dot ibm dot com>, GCC Development <gcc at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 4 Nov 2014 12:20:07 +0100
- 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> <CAOyqgcUqnQkToEGVYAdoD19mwaa6L1SX8KeDbESO+jSC-DmEGw at mail dot gmail dot com>
On Tue, Nov 4, 2014 at 1:00 AM, Ian Taylor <iant@golang.org> wrote:
> 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.
I have checked that the patch fixes alpha bootstrap with libgo.
Thanks,
Uros.