On 02/26/2016 12:41 PM, Segher Boessenkool wrote: > On Thu, Feb 25, 2016 at 09:08:32PM -0800, Richard Henderson wrote: >> It's the simplify-rtx.c portion of the patch that fixes the i686 regression. >> >> In the PR, Alan raises some good points, but I don't believe that we can >> address those for gcc6. A new rtl reassoc optimization that takes loop >> invariance into account will have to wait. >> >> But we do need to take care of the rs6000 ice that results, and that's the >> bulk of the patch -- allowing CA to be sorted to any register of the plus >> chain. > > The rs6000 change is much too risky for stage 4 as well in my opinion. > It also seems it will regress codegen (for which we have no testcases > yet, sigh). > > What is the rs6000 ICE?
The simplify-rtx.c patch causes (reg:M ca) to get sorted to a different spot in the (plus (plus r1 r2) r3) chain than the rs6000 backend expects, producing an ICE due to an unrecognizable insn. The rs6000 patch changes the backend to allow (reg:M ca) to appear exactly once in (plus (plus r1 r2) r3). And we do have a test case for it -- the ICE. How do you imagine the rs6000 change will regress codegen? >> ca_operand doesn't work as written, since CA_REGNO is not an available >> register, and thus doesn't satisfy register_operand. > > Yes, curious. But when it was written it *did* match. Huh. It probably started to fail with r181760, back in 2011. But the predicate wasn't much used in the rs6000 backend, so I guess it hadn't really mattered. >> Is there any particular reason that subf<>3_carry_in_m1 was written with >> minus rather than plus like all of the other patterns? > > It's what RTL simplification comes up with, so it is the only thing > combine tries. The same reason why there are separate patterns for 0 > and -1 at all. Well, it appears to use the new pattern just as well. A few trivial tests are in fact simplified as one would expect during combine. r~
