Re: [PATCH] [LRA] Do not use eliminable registers for spilling

2019-11-08 Thread Vladimir Makarov



On 2019-11-07 12:28 p.m., Kwok Cheung Yeung wrote:

Hello

On AMD GCN, I encountered the following situation in the following 
testcases using the compilation flags '-O2 -ftracer -fsplit-paths':


libgomp.oacc-fortran/reduction-1.f90
libgomp.oacc-fortran/reduction-2.f90
libgomp.oacc-fortran/reduction-3.f90
gcc.c-torture/execute/ieee/pr50310.c

- LRA decides to spill a register to s14 (which is used for the hard 
frame pointer, but is not in use due to the -fomit-frame-pointer 
implied by -O2). The reload dump has:


  Spill r612 into hr14
...
(insn 597 711 712 2 (set (reg:BI 129 scc [612])
    (ne:BI (reg:SI 2 s2 [684])
    (const_int 0 [0]))) "reduction-1.f90":22:0 23 {cstoresi4}
 (nil))
...
(insn 710 713 598 2 (set (reg:BI 14 s14)
    (reg:BI 160 v0 [685])) "reduction-1.f90":22:0 3 {*movbi}
 (nil))

- Later on, LRA decides to allocate s14 to a pseudo:

 Assigning to 758 (cl=ALL_REGS, orig=758, freq=388, tfirst=758, 
tfreq=388)...

   Assign 14 to subreg reload r758 (freq=388)
...
(insn 801 786 787 34 (set (reg:BI 14 s14 [758])
    (reg:BI 163 v3 [758])) 3 {*movbi}
 (nil))

- But then the next BB reloads the value previously spilled into s14, 
which has been clobbered by previous instruction:


(insn 733 144 732 9 (set (reg:BI 163 v3 [706])
    (reg:BI 14 s14)) 3 {*movbi}
 (nil))

A similar issue has been dealt with in the past in PR83327, which was 
fixed in r258093. However, it does not work here - s14 is not marked 
as conflicting with pseudo 758.


This is because s14 is in eliminable_regset - if 
HARD_FRAME_POINTER_IS_FRAME_POINTER is false, 
ira_setup_eliminable_regset puts HARD_FRAME_POINTER_REGNUM into 
eliminable_regset even if the frame pointer is not needed (Why is 
this? It seems to have been that way since IRA was introduced).


  I don't remember exactly why.  It was long time ago (12 years).  I 
suspect it was related to the fact that IRA worked with reload first and 
LRA was added much later and reload communicated differently to IRA than 
to the old global RA.  I guess we could try to change it and see what 
happens.



At the beginning of process_bb_lives (in lra-lives.c), 
eliminable_regset is ~ANDed out of hard_regs_live, so even if s14 is 
in the live-outs of the BB, it will be removed from consideration when 
registering conflicts with pseudos in the BB.


(As an aside, the liveness of eliminable spill registers would 
previously have been updated by make_hard_regno_live and 
make_hard_regno_dead, but as of r276440 '[LRA] Don't make eliminable 
registers live (PR91957)' this is no longer the case.)


Given that conflicts with registers in eliminable_regset is not 
tracked, I believe the easiest fix is simply to prevent eliminable 
registers from being used as spill registers.


Built and tested on AMD GCN with no regressions.

I've bootstrapped it on x86_64, but there is no point testing on it 
ATM as TARGET_SPILL_CLASS was disabled in r237792.


Okay for trunk?



Yes, the patch is ok.

Thank you for the patch and good explanation of the problem.




    [LRA] Do not use eliminable registers for spilling

    2019-11-07  Kwok Cheung Yeung  

    gcc/
    * lra-spills.c (assign_spill_hard_regs): Do not spill into
    registers in eliminable_regset.

diff --git a/gcc/lra-spills.c b/gcc/lra-spills.c
index 0068e52..54f76cc 100644
--- a/gcc/lra-spills.c
+++ b/gcc/lra-spills.c
@@ -283,6 +283,8 @@ assign_spill_hard_regs (int *pseudo_regnos, int n)
   for (k = 0; k < spill_class_size; k++)
 {
   hard_regno = ira_class_hard_regs[spill_class][k];
+  if (TEST_HARD_REG_BIT (eliminable_regset, hard_regno))
+    continue;
   if (! overlaps_hard_reg_set_p (conflict_hard_regs, mode, 
hard_regno))

 break;
 }




[PATCH] [LRA] Do not use eliminable registers for spilling

2019-11-07 Thread Kwok Cheung Yeung

Hello

On AMD GCN, I encountered the following situation in the following 
testcases using the compilation flags '-O2 -ftracer -fsplit-paths':


libgomp.oacc-fortran/reduction-1.f90
libgomp.oacc-fortran/reduction-2.f90
libgomp.oacc-fortran/reduction-3.f90
gcc.c-torture/execute/ieee/pr50310.c

- LRA decides to spill a register to s14 (which is used for the hard 
frame pointer, but is not in use due to the -fomit-frame-pointer implied 
by -O2). The reload dump has:


  Spill r612 into hr14
...
(insn 597 711 712 2 (set (reg:BI 129 scc [612])
(ne:BI (reg:SI 2 s2 [684])
(const_int 0 [0]))) "reduction-1.f90":22:0 23 {cstoresi4}
 (nil))
...
(insn 710 713 598 2 (set (reg:BI 14 s14)
(reg:BI 160 v0 [685])) "reduction-1.f90":22:0 3 {*movbi}
 (nil))

- Later on, LRA decides to allocate s14 to a pseudo:

	 Assigning to 758 (cl=ALL_REGS, orig=758, freq=388, tfirst=758, 
tfreq=388)...

   Assign 14 to subreg reload r758 (freq=388)
...
(insn 801 786 787 34 (set (reg:BI 14 s14 [758])
(reg:BI 163 v3 [758])) 3 {*movbi}
 (nil))

- But then the next BB reloads the value previously spilled into s14, 
which has been clobbered by previous instruction:


(insn 733 144 732 9 (set (reg:BI 163 v3 [706])
(reg:BI 14 s14)) 3 {*movbi}
 (nil))

A similar issue has been dealt with in the past in PR83327, which was 
fixed in r258093. However, it does not work here - s14 is not marked as 
conflicting with pseudo 758.


This is because s14 is in eliminable_regset - if 
HARD_FRAME_POINTER_IS_FRAME_POINTER is false, 
ira_setup_eliminable_regset puts HARD_FRAME_POINTER_REGNUM into 
eliminable_regset even if the frame pointer is not needed (Why is this? 
It seems to have been that way since IRA was introduced). At the 
beginning of process_bb_lives (in lra-lives.c), eliminable_regset is 
~ANDed out of hard_regs_live, so even if s14 is in the live-outs of the 
BB, it will be removed from consideration when registering conflicts 
with pseudos in the BB.


(As an aside, the liveness of eliminable spill registers would 
previously have been updated by make_hard_regno_live and 
make_hard_regno_dead, but as of r276440 '[LRA] Don't make eliminable 
registers live (PR91957)' this is no longer the case.)


Given that conflicts with registers in eliminable_regset is not tracked, 
I believe the easiest fix is simply to prevent eliminable registers from 
being used as spill registers.


Built and tested on AMD GCN with no regressions.

I've bootstrapped it on x86_64, but there is no point testing on it ATM 
as TARGET_SPILL_CLASS was disabled in r237792.


Okay for trunk?

Kwok Yeung


[LRA] Do not use eliminable registers for spilling

2019-11-07  Kwok Cheung Yeung  

gcc/
* lra-spills.c (assign_spill_hard_regs): Do not spill into
registers in eliminable_regset.

diff --git a/gcc/lra-spills.c b/gcc/lra-spills.c
index 0068e52..54f76cc 100644
--- a/gcc/lra-spills.c
+++ b/gcc/lra-spills.c
@@ -283,6 +283,8 @@ assign_spill_hard_regs (int *pseudo_regnos, int n)
   for (k = 0; k < spill_class_size; k++)
{
  hard_regno = ira_class_hard_regs[spill_class][k];
+ if (TEST_HARD_REG_BIT (eliminable_regset, hard_regno))
+   continue;
  if (! overlaps_hard_reg_set_p (conflict_hard_regs, mode, hard_regno))
break;
}