On Tue, 2020-04-07 at 21:31 +0200, Martin Jambor wrote:
> Hi Jeff,
> 
> On Tue, Apr 07 2020, Jeff Law wrote:
> > 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.
> 
> I did not commit the patch.  I waited to look at test results from other
> platforms and saw the failure there and so did not proceed.  So unless
> you tested it independently, it's not caused by my patch.
ohhh, interesting, no I did not test that independently...  I'll put it back in
the queue of things to investigate.

jeff
> 

Reply via email to