This is the mail archive of the gcc-patches@gcc.gnu.org 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: [RFC] Fix SRA with respect of BIT_FIELD_REF


Hi,

On Fri, Jun 01, 2012 at 11:31:20AM +0200, Richard Guenther wrote:
> On Fri, Jun 1, 2012 at 6:02 AM, Andrew Pinski
> <andrew.pinski@caviumnetworks.com> wrote:
> > Hi,
> > ÂWhen I modified GCC to change the majority of bitfield accesses
> > which were done via component ref to BIT_FIELD_REF, SRA messes up
> > because when it does the replacement it still tries to use the
> > BIT_FIELD_REF except it never places the old value in the struct for
> > the BIT_FIELD_REF to work correctly.
> >
> > This patch fixes the problem by expanding what BIT_FIELD_REF does
> > internally for both writing and reading. ÂNote we can't use
> > BIT_FIELD_REF directly on the left hand since we don't support partial
> > writes yet (except for vector and complex types).
> >
> > This is only a RFC since I don't know a way to reproduce this bug on
> > the trunk. I tested it on x86_64-linux-gnu with no regressions.
> 
> I'd rather disqualify SRA of BIT_FIELD_REFs on the LHS for now.  My goal
> was to enable SRA of bitfields using the DECL_BIT_FIELD_REPRESENTATIVE
> work - something you go against with replacing them with BIT_FIELD_REFs.

SRA of bit-fields works now, it is just rather simple and can't
optimize the bit-field accesses as we probably should.  Therefore I am
all for using DECL_BIT_FIELD_REPRESENTATIVEs and looked at those
patches with interest, I'm just wondering why we'd want to do it for
non-addressable structures only, which is something SRA would imply if
this "lowering" was part of it.

BIT_FIELD_REFs of non-vectors on the LHS are not much tested, I'm
afraid. I it is quite possible it does not do the right thing.
Nevertheless, making regions accessed through them unscalarizable
might also be an option, if it does not induce significant penalties
anywhere.

Thanks,

Martin

> 
> You'd replace
> 
>   x = a.b;
> 
> with
> 
>   tem = a.<representative for b>;
>   x = BIT_FIELD_REF <tem, ....>;
> 
> and for stores with a read-modify-write sequence, possibly adding
> the bitfield-insert tree I proposed some time ago.  Replace
> 
>  a.b = x;
> 
> with
> 
>  tem = a.<representative for b>
>  tem = BIT_FIELD_EXPR <tem, x, ...>;
>  a.<representative for b> = tem;
> 
> and I'd do this replacement in SRA for now whenever it would decide to
> SRA a bitfield.
> 
> Richard.
> 
> > Thanks,
> > Andrew Pinski
> >
> > ChangeLog:
> > * tree-sra.c (sra_modify_expr): Handle BIT_FIELD_REF specially if we
> > are doing a replacement of the struct with one variable.


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