[PATCH] sra: Fix sra_modify_expr handling of partial writes (PR 94482)

Jeff Law law@redhat.com
Tue Apr 7 19:08:37 GMT 2020


On Tue, 2020-04-07 at 18:25 +0200, Martin Jambor wrote:
> Hi,
> 
> On Tue, Apr 07 2020, Richard Biener wrote:
> > On Tue, 7 Apr 2020, Martin Jambor wrote:
> > 
> > > Hi,
> > > 
> > > when sra_modify_expr is invoked on an expression that modifies only
> > > part of the underlying replacement, such as a BIT_FIELD_REF on a LHS
> > > of an assignment and the SRA replacement's type is not compatible with
> > > what is being replaced (0th operand of the B_F_R in the above
> > > example), it does not work properly, basically throwing away the partd
> > > of the expr that should have stayed intact.
> > > 
> > > This is fixed in two ways.  For BIT_FIELD_REFs, which operate on the
> > > binary image of the replacement (and so in a way serve as a
> > > VIEW_CONVERT_EXPR) we just do not bother with convertsing.  For
> > > REALPART_EXPRs and IMAGPART_EXPRs, we insert a VIEW_CONVERT_EXPR under
> > > the complex partial access expression, which is OK even in a LHS of an
> > > assignment (and other write contexts) because in those cases the
> > > replacement is not going to be a giple register anyway.
> > > 
> > > The testcase for handling REALPART_EXPR and IMAGPART_EXPR is a bit
> > > fragile because SRA prefers complex and vector types over anything else
> > > (and in between the two it decides based on TYPE_UID which in my testing
> > > today always preferred complex types) and even when GIMPLE is wrong I
> > > often still did not get failing runs, so I only run it at -O1 (which is
> > > the only level where the the test fails for me).
> > > 
> > > Bootstrapped and tested on x86_64-linux, bootstrap and testing on
> > > i686-linux and aarch64-linux underway.
> > > 
> > > OK for trunk (and subsequently for release branches) if it passes?
> > 
> > OK.
> 
> I must have been already half asleep when writing that it passed
> testing.  It did not, testsuite/gcc.c-torture/compile/pr42196-3.c fails
> even on x86_64, because fwprop happily combines
Yea, my tester complained about that on msp430-elf as well.  There's other recent
failures on msp430, ft32 and h8300 that I'm digging into now.
> 

Jeff



More information about the Gcc-patches mailing list