On Fri, 3 Feb 2012, Richard Guenther wrote:
> On Fri, 3 Feb 2012, Richard Guenther wrote:
>
> > On Thu, 2 Feb 2012, Aldy Hernandez wrote:
> >
> > > Linus Torvalds <torva...@linux-foundation.org> writes:
> > >
> > > > Seriously - is there any real argument *against* just using the base
> > > > type as a hint for access size?
> > >
> > > If I'm on the hook for attempting to fix this again, I'd also like to
> > > know if there are any arguments against using the base type.
> >
> > Well, if you consider
> >
> > struct {
> > int i : 1;
> > char c;
> > };
> >
> > then you'll realize that 'i' has SImode (and int type) but the
> > underlying bitfield has only 1 byte size (thus, QImode) and
> > 'c' starts at offset 1.
> >
> > So no, you cannot use the base type either.
> >
> > I've playing with the following patch yesterday, which computes
> > an "underlying object" for all bitfields and forcefully lowers
> > all bitfield component-refs to use that underlying object
> > (just to check correctness, it doesn't generate nice code as
> > BIT_FIELD_REF on memory is effectively resulting in the same
> > code as if using the bitfield FIELD_DECLs directly - we'd
> > need to explicitely split things into separate stmts with RMW
> > cycles).
> >
> > You should be able to re-use the underlying object compute though
> > (and we can make it more intelligent even) during expansion
> > for the C++ memory model (and in fact underlying object compute
> > might just do sth different dependent on the memory model in
> > effect).
> >
> > Disclaimer: untested.
>
> The following works (roughly, still mostly untested). SRA needs
> a fix (included) and the gimplify.c hunk really only shows what
> we are supposed to be able to do (access the representative).
> As-is SRA could now do a nice job on bitfields, but that needs
> some changes - or we lower all bitfield ops in some extra pass
> (if not then expand would need to be changed to look at the
> representatives instead).
>
> Still the idea is to compute all these things up-front during
> type layout instead of re-discovering them at each bitfield
> access we expand in get_bit_range. And we can use that information
> consistently across passes.
>
> We should of course try harder to avoid adding a new field to
> struct tree_field_decl - DECL_INITIAL came to my mind, but
> the C frontend happens to use that for bitfields ... while
> it probably could as well use lang_type.enum_{min,max}?
>
> Comments?
Funnily C++ uses tail-padding of base types to pack bitfields
and thus I run into
gcc_assert (maxbitsize % BITS_PER_UNIT == 0);
Testcase is for example g++.dg/abi/bitfield5.C, bit layout annotated:
struct A {
virtual void f();
int f1 : 1; <--- bit 64
};
struct B : public A {
int f2 : 1; // { dg-warning "ABI" } <--- bit 65
int : 0;
int f3 : 4;
int f4 : 3;
};
maybe it was a bug (above happens with -fabi-version=1 only),
but certainly an ABI may specify that we should do that packing.
What does the C++ memory model say here? (incidentially that's
one case I was worried about when reviewing your patches,
just I didn't think of _bitfield_ tail-packing ... ;)).
I suppose I could just force the bitfield region to start
at a byte boundary.
Richard.
