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