[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

Tom de Vries  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |FIXED
   Assignee|unassigned at gcc dot gnu.org  |vries at gcc dot gnu.org
   Target Milestone|--- |8.0

--- Comment #14 from Tom de Vries  ---
Patch tested on MIPS16 and committed.

Marking resolved-fixed.

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

--- Comment #13 from Tom de Vries  ---
Author: vries
Date: Thu Mar  1 05:51:08 2018
New Revision: 258093

URL: https://gcc.gnu.org/viewcvs?rev=258093=gcc=rev
Log:
Fix liveness analysis in lra for spilled-into hard regs

2018-03-01  Tom de Vries  

PR rtl-optimization/83327
* lra-int.h (hard_regs_spilled_into): Declare.
* lra.c (hard_regs_spilled_into): Define.
(init_reg_info): Init hard_regs_spilled_into.
* lra-spills.c (assign_spill_hard_regs): Update hard_regs_spilled_into.
* lra-lives.c (make_hard_regno_born, make_hard_regno_dead)
(process_bb_lives): Handle hard_regs_spilled_into.
(lra_create_live_ranges_1): Before doing liveness propagation, clear
regs in all_hard_regs_bitmap if set in hard_regs_spilled_into.

Modified:
trunk/gcc/ChangeLog
trunk/gcc/lra-int.h
trunk/gcc/lra-lives.c
trunk/gcc/lra-spills.c
trunk/gcc/lra.c

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[mpf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

mpf at gcc dot gnu.org changed:

   What|Removed |Added

 CC||mpf at gcc dot gnu.org

--- Comment #12 from mpf at gcc dot gnu.org ---
(In reply to Tom de Vries from comment #11)
> (In reply to Vladimir Makarov from comment #10)
> > Any news about the patch testing on MIPS.  It would be nice to move forward
> > with the PR.
> 
> Pinged Matthew here: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01392.html

Hi Tom,

Sorry for my general lack of engagement I'm struggling to do much at all to
support the community at the moment.

I have set off a couple of builds to target MIPS16; I don't have any recipe for
bootstrapping with MIPS16 enabled so the most efficient way I can give some
assurance is just the GCC testsuite run cross compiled with a simulator. I've
started two build of GCC without and with the patch and will aim to report back
on the testsuite runs as soon as I can.

Matthew

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

--- Comment #11 from Tom de Vries  ---
(In reply to Vladimir Makarov from comment #10)
> Any news about the patch testing on MIPS.  It would be nice to move forward
> with the PR.

Pinged Matthew here: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg01392.html

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vmakarov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

--- Comment #10 from Vladimir Makarov  ---
Any news about the patch testing on MIPS.  It would be nice to move forward
with the PR.

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

Tom de Vries  changed:

   What|Removed |Added

 CC||matthew.fortune at imgtec dot 
com

--- Comment #9 from Tom de Vries  ---
(In reply to Tom de Vries from comment #8)
> (In reply to Tom de Vries from comment #7)
> > (In reply to Tom de Vries from comment #6)
> > > Submitted: https://gcc.gnu.org/ml/gcc-patches/2017-12/msg01030.html
> > 
> > Approved. Todo: testing.
> 
> Bootstrap and reg-test on x86_64 done.
> 
> Todo: mips testing.

Asked Matthew to help us out with the testing:
https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00503.html .

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

--- Comment #8 from Tom de Vries  ---
(In reply to Tom de Vries from comment #7)
> (In reply to Tom de Vries from comment #6)
> > Submitted: https://gcc.gnu.org/ml/gcc-patches/2017-12/msg01030.html
> 
> Approved. Todo: testing.

Bootstrap and reg-test on x86_64 done.

Todo: mips testing.

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

--- Comment #7 from Tom de Vries  ---
(In reply to Tom de Vries from comment #6)
> Submitted: https://gcc.gnu.org/ml/gcc-patches/2017-12/msg01030.html

Approved. Todo: testing.

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

Tom de Vries  changed:

   What|Removed |Added

   Keywords||patch

--- Comment #6 from Tom de Vries  ---
Submitted: https://gcc.gnu.org/ml/gcc-patches/2017-12/msg01030.html

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

--- Comment #5 from Tom de Vries  ---
tentative patch mentioned here:
https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00813.html

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

Tom de Vries  changed:

   What|Removed |Added

  Attachment #42850|0   |1
is obsolete||

--- Comment #4 from Tom de Vries  ---
Created attachment 42864
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42864=edit
Updated tentative patch

> I then added fixes in make_hard_regno_{born,dead} in the same style as the 
> fix 
> for PR82353, and that allowed the live info to be propagated.

I was missing this part, which makes sure that we don't mask out the
hard_regs_spilled_into during propagation:
...
@@ -1320,6 +1338,11 @@ lra_create_live_ranges_1 (bool all_p, bool dead_insn_p)
}   
   /* As we did not change CFG since LRA start we can use   
 DF-infrastructure solver to solve live data flow problem.  */  
+  for (int i = 0; i < FIRST_PSEUDO_REGISTER; ++i)  
+   {   
+ if (TEST_HARD_REG_BIT (hard_regs_spilled_into, i))
+   bitmap_clear_bit (_hard_regs_bitmap, i);
+   }   
   df_simple_dataflow   
(DF_BACKWARD, NULL, live_con_fun_0, live_con_fun_n, 
 live_trans_fun, _blocks,   
...

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

Tom de Vries  changed:

   What|Removed |Added

  Attachment #42817|0   |1
is obsolete||

--- Comment #3 from Tom de Vries  ---
Created attachment 42850
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42850=edit
Tentative trunk patch

I've re-enabled i86_spill_class on trunk, added a printf when the optimization
is effective and build gcc on my laptop (with kaby lake cpu) with march=haswell
enabled by default.

I investigated the trigger:
...
  Spill r87 into hr24 for fn __eqtf2 in libgcc/soft-fp/eqtf2.c
...
and observed that the life-range r87 is stretched over different basic blocks.

Then I added a "lra_create_live_ranges (lra_reg_spill_p, true)" after lra_spill
to force the live range analysis to run.

I observed that the live range analysis was only run once, meaning that the
hr24 usage is not propagated, which is AFAIU a reproduction of the root cause
of this PR.

I added code to the end of process_bb_lives to trigger the second live range
analysis:
...
@@ -1049,6 +1049,24 @@ process_bb_lives (basic_block bb, int _point, bool
dead_insn_p)
check_pseudos_live_through_calls (j, last_call_used_reg_set);
 }

+  for (i = 0; i < FIRST_PSEUDO_REGISTER; ++i)
+{
+  if (!TEST_HARD_REG_BIT (hard_regs_live, i))
+   continue;
+
+  if (!TEST_HARD_REG_BIT (hard_regs_spilled_into, i))
+   continue;
+
+  if (bitmap_bit_p (df_get_live_in (bb), i))
+   continue;
+
+  live_change_p = true;
+  if (lra_dump_file)
+   fprintf (lra_dump_file,
+"  hard reg r%d is added to live at bb%d start\n", i,
bb->index);
+  bitmap_set_bit (df_get_live_in (bb), i);
+}
+
   if (need_curr_point_incr)
 next_program_point (curr_point, freq);

...
and ran into the assert at the end of lra_create_live_ranges because although
the register is noted as changing liveness, it's still not propagated, and we
don't reach fixed point.

I then added fixes in make_hard_regno_{born,dead} in the same style as the fix
for PR82353, and that allowed the live info to be propagated.

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

--- Comment #2 from Tom de Vries  ---
Created attachment 42817
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=42817=edit
Demonstrator patch

Using this demonstrator patch, I managed to fix the failure and run the
test-case successfully.

I'm not sure if the "AND_COMPL_HARD_REG_SET (hard_regs_live,
lra_no_alloc_regs)" bit (which I borrowed from
ira-lives.c:process_bb_node_lives) is necessary, this might be due to an
problem in the gcn port.

[Bug rtl-optimization/83327] Spilling into hard regs not taken into account in lra liveness analysis

[vries at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327

Tom de Vries  changed:

   What|Removed |Added

 CC||vmakarov at gcc dot gnu.org

--- Comment #1 from Tom de Vries  ---
> However, a bit later in lra we decide to assign r94,r95 to DImode pseudo 833:

This happens because r95 is not marked in the conflict_hard_regs of
lra_reg_info[833] during liveness analysis.

There's code in make_hard_regno_born to set the reg in the conflict_hard_regs
of live pseudos, but at the point that r95 becomes live, the r833 pseudo is not
live.

Then there's code in mark_pseudo_live to set the hard_regs_live in the
conflict_hard_regs of the pseudo, but at the point that r833 becomes live, r95
is not set in hard_regs_live, due to the fact that it's not set in
df_get_live_out (bb2).

OTOH, r95 is set in hard_regs_live at the end of processing bb3. But there's no
effort done to communicate the hard_regs_live findings to df_get_live_in (bb3),
to allow df propagation to df_get_live_out (bb2).

Furthermore, the df functions live_con_fun_0, live_con_fun_n and live_trans_fun
are explicitly setup to ignore hard regs.