On Thu, Mar 12, 2020 at 12:47:04PM -0600, Jeff Law wrote: > On Thu, 2020-03-12 at 13:23 -0500, Segher Boessenkool wrote: > > > 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.
Ah, I see. So this is exposed on Power by the TOC stuff, I guess? CSE sees a TOC set parallel with a call as a no-op because it is set to the same value (an unspec, not an unspec_volatile) that GCC can derive is already in the TOC reg? Or is this some other case? The change sounds fine, fwiw. > One could argue that find_sets_in_insn should be fixed as well. I'd be > worried > about fallout from that. Should it ignore all SETs in parallel with a call? Or what do you want to fix there? Segher
