On Thu, 2020-03-12 at 13:23 -0500, Segher Boessenkool wrote: > > > > I don't know if this patch makes matters worse or not. It doesn't seem > > > suitable for stage 4 though. And Richard said the cse.c part breaks > > > rs6000, if that is true, yes I do object ;-) > > The rs6000 port breakage is trivial to fix. In fact, I did so and ran it > > through > > my tester, which includes ppc64le and ppc64 a slew of *-elf targets x86 > > native > > and more. > > I don't see anything rs6000 below? Is it just this generic code? It's just generic code. THe rs6000 issue is fixed by the !CALL_P condition.
> > > @@ -5324,9 +5324,11 @@ cse_insn (rtx_insn *insn) > > } > > > > /* Similarly, lots of targets don't allow no-op > > - (set (mem x) (mem x)) moves. */ > > + (set (mem x) (mem x)) moves. Even (set (reg x) (reg x)) > > + might be impossible for certain registers (like CC registers). */ > > else if (n_sets == 1 > > - && MEM_P (trial) > > + && ! CALL_P (insn) > > + && (MEM_P (trial) || REG_P (trial)) > > && MEM_P (dest) > > && rtx_equal_p (trial, dest) > > && !side_effects_p (dest) > > This adds the !CALL_P (no space btw) condition, why is that? Because n_sets is not valid for CALL_P insns which resulted in a failure on ppc. See find_sets_in_insn which ignores the set on the LHS of a call. So imagine if we had a nop register set in parallel with a (set (reg) (call ...)). We'd end up deleting the entire PARALLEL which is obviously wrong. One could argue that find_sets_in_insn should be fixed as well. I'd be worried about fallout from that. jeff
