[PATCH] lra: Clobbers in a parallel are earlyclobbers (PR83245)

[PATCH] lra: Clobbers in a parallel are earlyclobbers (PR83245)

[Segher Boessenkool]

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?


Segher


2017-12-02  Segher Boessenkool  <segher@kernel.crashing.org>

	* lra.c (collect_non_operand_hard_regs): Treat clobbers of non-operand
	hard registers as earlyclobber, also if not in an asm.

---
 gcc/lra.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

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);
-- 
1.8.3.1

Re: [PATCH] lra: Clobbers in a parallel are earlyclobbers (PR83245)

[Vladimir Makarov]

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.

Re: [PATCH] lra: Clobbers in a parallel are earlyclobbers (PR83245)

[Andreas Krebbel]

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-