This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: reverse bitfield patch
- From: DJ Delorie <dj at redhat dot com>
- To: Richard Guenther <richard dot guenther at gmail dot com>
- Cc: gcc at gcc dot gnu dot org
- Date: Wed, 2 Jul 2014 01:10:13 -0400
- Subject: Re: reverse bitfield patch
- Authentication-results: sourceware.org; auth=none
- References: <201210022207 dot q92M7NSk032449 at greed dot delorie dot com> <CAFiYyc2sCvqJhMh5nFjjdBJUJhRW_EiX5_iOyt1m=3zEZ+anzQ at mail dot gmail dot com> <201210041838 dot q94Icl9V012730 at greed dot delorie dot com> <CAFiYyc1EMf-u+QdExXFXAbm-vaR6iECKdBqgKndCzKeywaq3mA at mail dot gmail dot com> <201210051815 dot q95IFgOQ011559 at greed dot delorie dot com> <CAFiYyc07_wcJdeOUygDcGVhrfw=CcTaZ8jgCAqHPUM6g=CWbbA at mail dot gmail dot com>
Revisiting an old thread, as I still want to get this feature in...
https://gcc.gnu.org/ml/gcc/2012-10/msg00099.html
> >> Why do you need to change varasm.c at all? The hunks seem to be
> >> completely separate of the attribute.
> >
> > Because static constructors have fields in the original order, not the
> > reversed order. Otherwise code like this is miscompiled:
>
> Err - the struct also has fields in the original order - only the bit positions
> of the fields are different because of the layouting option.
The order of the field decls in the type (stor-layout.c) is not
changed, only the bit position information. The order here *can't* be
changed, because the C language assumes that parameters, initializers,
etc are presented in the same order as the original declaration,
regardless of the target-specific layout.
When the program includes an initializer:
> > struct foo a = { 1, 2, 3 };
The order of 1, 2, and 3 need to correspond to the order of the
bitfields in 'a', so we can change neither the order of the bitfields
in 'a' nor the order of constructor fields.
However, when we stream the initializer out to the .S file, we need to
pack the bitfields in the right sequence to generate the right bit
patterns in the final output image. The code in varasm.c exists to
make sure that the initializers for bitfields are written/packed in
the correct order, to correspond to the bitfield positions. I.e. the
1,2,3 initializer needs to be written to the .S file as either 0x0123
or 0x3210 depending on the bit positions.
In neither case do we change the order of the fields in the type
itself, i.e. the array/chain order.
> And you expect no other code looks at fields of a structure and its
> initializer? It's bad to keep this not in-sync. Thus I don't think it's
> viable to re-order fields just because bit allocation is reversed.
The fields are in sync. The varasm.c change sorts the elements as
they're being output into the byte stream in the .S, it doesn't sort
the field definitions themselves.
> > + /* If the bitfield-order attribute has been used on this
> > + structure, the fields might not be in bit-order. In that
> > + case, we need a separate representative for each
> > + field. */
> > The typical use-case for this feature is memory-mapped hardware, where
> > pessimum access is preferred anyway.
>
> I doubt that, looking at constraints for strict volatile bitfields.
The code that handles representatives requires (via an assert, IIRC)
that the bit offsets within a representative be in ascending order.
I.e. gcc ICEs if I don't bypass this. In the case of volatile
bitfields, which would be the typical use case for a reversed
bitfield, the access mode is going to match the type size regardless,
so performance is not changed by this patch.