This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: reverse bitfield patch

Revisiting an old thread, as I still want to get this feature in...

> >> 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.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]