Hi!
On Tue, May 02, 2023 at 05:20:49PM +0100, Roger Sayle wrote:
> On 02 May 2023 14:49, Segher Boessenkool wrote:
> Then combine inserts an additional copy:
Combine makes sure a pseudo-to-pseudo move remains. Without that,
combine will seize part of RA's job, and butcher it. It has always done
that, but the 2-2 combine patches made it clearer than before.
> (insn 17 16 18 2 (clobber (reg/v:SI 26 [ x ])) "../../shiftsi.c":4:41 -1
> (nil))
> (insn 18 17 19 2 (set (subreg:HI (reg/v:SI 26 [ x ]) 0)
> (reg:HI 30)) "../../shiftsi.c":4:41 6 {movhi_internal}
> (nil))
> (insn 19 18 3 2 (set (subreg:HI (reg/v:SI 26 [ x ]) 2)
> (reg:HI 31 [+2 ])) "../../shiftsi.c":4:41 6 {movhi_internal}
> (nil))
> I don't think it's a problem/fault with the machine description, but
> how the clobber above is being interpreted by the different register
> allocators.
Insn 17 is trivially dead code and should have been removed. It
arguably should not have been created in the first place. Why do we do
that, what is the purpose?
> Back in August 2022, I submitted a x86_64 backend patch that used a
> peephole2 to eliminate this type of (subreg-lower) generated clobber:
> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599419.html
> but Uros was hesitant to accept this change, without an RTL expert
> explaining why the clobbers were being generated in the first place.
It makes sure any previous value in it is regarded as dead, but since
insns 18 and 19 clearly write to the whole register anyway, this is just
extra work to later undo. Or not undo, which is the problem here :-/
> Like my x86_64 patch above, this issue could also probably be cleaned
> up by a peephole2 for xstormy16 that straddled/removed the clobber.
Or simply not create such useless clobbers in the first place?
> My simplistic summary is that when a backend lowers a multi-word
> instruction with a splitter, it (typically) does so without introducing
> a clobber. If the clobbers generated by the middle-end's automatic
> lowering confuse the register allocator, then this is more efficient.
> Ideally, such clobbers should be avoided (if possible) and/or LRA
> improved to understand that they don't actually cause an allocno
> conflict. [but I'm in no way a register allocation expert].
Yup, I agree with all that. We should not create dead code, and not be
confused by dead code either :-)
Segher
