Message ID | 7ae9f258fb2ffe40746ed9ec3592e8c0ca5d9575.1512240911.git.segher@kernel.crashing.org |
---|---|
State | New |
Headers | show |
Series | lra: Clobbers in a parallel are earlyclobbers (PR83245) | expand |
On 12/02/2017 01:59 PM, Segher Boessenkool wrote: > The documentation (rtl.texi) says: > > When a @code{clobber} expression for a register appears inside a > @code{parallel} with other side effects, the register allocator > guarantees that the register is unoccupied both before and after that > insn if it is a hard register clobber. > > and at least the rs6000 backend relies on that (see PR83245). This > patch restores that behaviour. > > Registers that are also used as operands in the instruction are not > treated as earlyclobber, so such insns also still work (PR80818, an > s390 testcase). > > Tested on powerpc64-linux {-m32,-m64}, also tested with a s390 cross. > > Andreas, can you confirm this really still works after this patch? > Vlad, if so, is this okay for trunk? > > > Yes, it is ok for me in any case, even if it creates a trouble for s390 PR80818. Sorry for the inconvenience from my patch. I should have been more careful.
On 12/04/2017 01:13 AM, Vladimir Makarov wrote: > > > On 12/02/2017 01:59 PM, Segher Boessenkool wrote: >> The documentation (rtl.texi) says: >> >> When a @code{clobber} expression for a register appears inside a >> @code{parallel} with other side effects, the register allocator >> guarantees that the register is unoccupied both before and after that >> insn if it is a hard register clobber. >> >> and at least the rs6000 backend relies on that (see PR83245). This >> patch restores that behaviour. >> >> Registers that are also used as operands in the instruction are not >> treated as earlyclobber, so such insns also still work (PR80818, an >> s390 testcase). >> >> Tested on powerpc64-linux {-m32,-m64}, also tested with a s390 cross. >> >> Andreas, can you confirm this really still works after this patch? >> Vlad, if so, is this okay for trunk? >> >> >> > Yes, it is ok for me in any case, even if it creates a trouble for s390 > PR80818. Sorry for the inconvenience from my patch. I should have been > more careful. With the patch the original failure is back on s390 :( But I agree that the change is required. PR80818 needs to be solved differently. I think the problem with PR80818 is that in insns like: (insn 472 27815 25079 10 (parallel [ (set (reg:SI 7 %r7 [orig:1282 _2003 ] [1282]) (plus:SI (plus:SI (gtu:SI (reg:CCU 33 %cc) (const_int 0 [0])) (reg:SI 7 %r7 [orig:1256 _1967 ] [1256])) (reg:SI 2 %r2 [orig:1258 _1969 ] [1258]))) (clobber (reg:CC 33 %cc)) ]) "t.f90":48 1661 {*addsi3_alc} (nil)) r33 is NOT recognized as being used in an operand when it comes to recording early clobbers. The operand RTX is: (gtu:SI (reg:CCU 33 %cc) (const_int 0 [0])) The code at collect_non_operand_hard_regs only checks for exact matches with operand locations and therefore does not consider r33 an operand: for (i = 0; i < data->insn_static_data->n_operands; i++) if (! data->insn_static_data->operand[i].is_operator && x == data->operand_loc[i]) /* It is an operand loc. Stop here. */ return list; -Andreas-
diff --git a/gcc/lra.c b/gcc/lra.c index f49c50a..0d76eac 100644 --- a/gcc/lra.c +++ b/gcc/lra.c @@ -888,14 +888,10 @@ collect_non_operand_hard_regs (rtx_insn *insn, rtx *x, list, OP_IN, false); break; case CLOBBER: - { - int code = INSN_CODE (insn); - /* We treat clobber of non-operand hard registers as early - clobber (the behavior is expected from asm). */ - list = collect_non_operand_hard_regs (insn, &XEXP (op, 0), data, - list, OP_OUT, code < 0); - break; - } + /* We treat clobber of non-operand hard registers as early clobber. */ + list = collect_non_operand_hard_regs (insn, &XEXP (op, 0), data, + list, OP_OUT, true); + break; case PRE_INC: case PRE_DEC: case POST_INC: case POST_DEC: list = collect_non_operand_hard_regs (insn, &XEXP (op, 0), data, list, OP_INOUT, false);