Jeff Law <l...@redhat.com> writes: > On 01/04/2018 02:28 PM, Richard Sandiford wrote: >> LRA has code to try to prevent cycling, by avoiding reloads that >> look too similar to the instruction being reloaded. E.g. if we >> have a R<-C move for some constant C, reloading the source with >> another R<-C move is unlikely to be a good idea. >> >> However, this safeguard unnecessarily triggered in tests like >> the one in the patch. We started with instructions like: >> >> (insn 12 9 13 5 (set (reg:DI 0 x0) >> (reg/f:DI 459)) "reg-alloc-1.c":18 47 {*movdi_aarch64} >> (expr_list:REG_EQUAL (symbol_ref:DI ("x00") [flags 0xc0] <var_decl >> 0x7f3c03c1f510 x00>) >> (nil))) >> >> where r459 didn't get allocated a register and is equivalent to >> constant x00. LRA would then handle it like this: >> >> Changing pseudo 459 in operand 1 of insn 12 on equiv `x00' >> 1 Non-pseudo reload: reject+=2 >> 1 Non input pseudo reload: reject++ >> alt=0,overall=609,losers=1,rld_nregs=1 >> [...] >> alt=13,overall=9,losers=1,rld_nregs=1 >> [...] >> Choosing alt 13 in insn 12: (0) r (1) w {*movdi_aarch64} >> >> In other words, to avoid loading the constant x00 into another GPR, >> LRA decided instead to move it into a floating-point register, >> then move that floating-point register into x0: >> >> Creating newreg=630, assigning class FP_REGS to r630 >> Set class ALL_REGS for r631 >> 12: x0:DI=r630:DI >> REG_EQUAL `x00' >> Inserting insn reload before: >> 815: r631:DI=high(`x00') >> 816: r630:DI=r631:DI+low(`x00') >> REG_EQUAL `x00' >> >> That's inefficient and doesn't really help to resolve a cycling >> problem, since the r630 destination of 816 needs to be reloaded into >> a GPR anyway. >> >> The cycling check already had an exception for source values that are >> the result of an elimination. This patch extends it to include the >> result of equivalence substitution. >> >> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. >> Also tested by comparing the before and after assembly output for at >> least one target per CPU directory. The targets most affected were: >> >> aarch64_be-linux-gn >> aarch64-linux-gnu >> powerpc-eabispe >> riscv32-elf >> riscv64-elf >> sparc64-linux-gnu >> sparc-linux-gnu >> spu-elf >> >> for which it improved code size overall. There were minor register >> renaming differences on some other targets. x86 and powerpc*-linux-gnu >> targets were unaffected. >> >> OK to install? >> >> Richard >> >> >> 2018-01-04 Richard Sandiford <richard.sandif...@linaro.org> >> >> gcc/ >> * lra-constraints.c (process_alt_operands): Test for the equivalence >> substitutions when detecting a possible reload cycle. >> >> gcc/testsuite/ >> * gcc.target/aarch64/reg-alloc-1.c: New test. > OK.
Thanks. > Presumably related to 83699? Do you want to reference it in the > ChangeLog? It's actually an unrelated problem that I'd originally seen on SVE. I guess I was just lucky to have two cycling bugs around the same time :-) Richard