On Sat, Jan 17, 2015 at 11:07:12AM +1030, Alan Modra wrote: > On Fri, Jan 16, 2015 at 11:03:24AM -0600, Segher Boessenkool wrote: > > On Fri, Jan 16, 2015 at 08:12:27PM +1030, Alan Modra wrote: > > > OK, so we need to fix this in the rs6000 backend, but it occurs to me > > > that cprop also has a bug here. It shouldn't be touching fixed hard > > > registers. > > > > Why not? It cannot allocate a fixed reg to a pseudo, but other than > > that there is nothing special about fixed regs; the transform is > > perfectly valid as far as I see. > > I didn't say that copying to a pseudo and using that was invalid.. > The bug I see is a mis-optimisation.
Ah, okay, good :-) This same mis-optimisation would happen if r1 was just some regular non-fixed register, hrm. Maybe something else in cprop needs some tuning up? > Also, the asm operands case that > do_local_cprop already rules out changing is very similar to fixed > regs. Would you argue that changing asm operands is also valid? :) A fixed reg in an asm_operands is a hard reg; a hard reg in an asm_operands (before reload) is a register asm variable. And we had better not change register variable asm arguments, since that is what we promise not to do with register variables. The case is not similar at all. > > It isn't a desirable transform in this case, but that is not true for > > fixed regs in general (just because the stack pointer is live everywhere). > > What's the point in extending the lifetime of some pseudo when you > know the original fixed register is available everywhere? That is my point: _if_ you know it is live all the time, or if there is no advantage to shortening the lifetime of the value in that fixed reg, then yes we should not propagate that value. But that is not true for all fixed regs. > Do you have > some concrete example in mind where this "optimisation" is beneficial? The CA_REG in rs6000 is a fixed register. It isn't a terribly good example because it cannot be propagated anyway, for other reasons; but it will hopefully help explain my point. So please pretend we can copy it to GPRs :-) [ The situation with the T bit on SH is similar, but I don't know the details there well enough. ] There is only one CA_REG. It is used in quite a few sequences. It contains a totally different value every time. Because there is only one such register the instruction sequences around it cannot be reordered very well. Propagating the value in such a not-so-very-fixed fixed reg helps reduce the lifetimes of the values in those regs, helps reordering, combining, scheduling, performance in general. If you are only concerned about the stack pointer, you could just check for that? But please add a comment in any case, saying why you exclude it (and ideally don't lump it in with tests that are needed for correctness). Cheers, Segher
