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 >