Re: RFA: patch to solve PR37535

[Richard Sandiford <rdsandiford at googlemail dot com>]

"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Sat, Sep 20, 2008 at 2:14 AM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> Vladimir Makarov <vmakarov@redhat.com> writes:
>>> The following patch solves the PR37535.  The analysis of the problem can
>>> be found on
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37535
>>
>>
>> I'm sorry to hear that you're so unhappy with that change.  I'm happy
>> to back it out if you'd prefer.  (Really.)
>>
>
> Richard, your patch fixes a few EH regressions. But it may not be complete.
> The fix for PR 37535 is incorrect and caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37598
>
> Richard, do you have a different fix for PR 37535?

I can try to come up with something tomorrow.  FWIW, I agree with
Andreas when he said:

> I think the second loop should only mark the registers dead which have
> been marked live before without being already live at that point.

So the moral equivalent of the attached (completely untested).

The patch is very inelegant though.  Although I think the complexity
of the current DF-based approach vs. the original note_stores one is
a wash, having to do something like the attached bears out Vlad's
complaint.

If we reverted the DF stuff, it would certainly be possible to fix the
EH problems using the original note_stores-based forward scan.

Richard


Index: gcc/gcc/ira-lives.c
===================================================================
--- gcc.orig/gcc/ira-lives.c	2008-09-20 18:21:21.000000000 +0100
+++ gcc/gcc/ira-lives.c	2008-09-20 18:40:50.000000000 +0100
@@ -212,9 +212,10 @@ clear_allocno_live (ira_allocno_t a)
 /* Mark the register referenced by use or def REF as live
    Store a 1 in hard_regs_live or allocnos_live for this register or
    the corresponding allocno, record how many consecutive hardware
-   registers it actually needs.  */
+   registers it actually needs.  Return true if at least some of
+   the register was dead before the call.  */
 
-static void
+static bool
 mark_ref_live (struct df_ref *ref)
 {
   rtx reg;
@@ -233,21 +234,25 @@ mark_ref_live (struct df_ref *ref)
       if (a != NULL)
 	{
 	  if (sparseset_bit_p (allocnos_live, ALLOCNO_NUM (a)))
-	    return;
+	    return false;
 	  set_allocno_live (a);
 	}
       make_regno_born (regno);
+      return true;
     }
   else if (! TEST_HARD_REG_BIT (ira_no_alloc_regs, regno))
     {
       int last = regno + hard_regno_nregs[regno][GET_MODE (reg)];
       enum reg_class cover_class;
+      bool some_dead_p;
 
+      some_dead_p = false;
       while (regno < last)
 	{
 	  if (! TEST_HARD_REG_BIT (hard_regs_live, regno)
 	      && ! TEST_HARD_REG_BIT (eliminable_regset, regno))
 	    {
+	      some_dead_p = true;
 	      cover_class = ira_class_translate[REGNO_REG_CLASS (regno)];
 	      if (cover_class != NO_REGS)
 		{
@@ -266,7 +271,9 @@ mark_ref_live (struct df_ref *ref)
 	    }
 	  regno++;
 	}
+      return some_dead_p;
     }
+  return false;
 }
 
 /* Return true if the definition described by DEF conflicts with the
@@ -639,7 +646,7 @@ process_bb_node_lives (ira_loop_tree_nod
       FOR_BB_INSNS_REVERSE (bb, insn)
 	{
 	  struct df_ref **def_rec, **use_rec;
-	  bool call_p;
+	  bool call_p, some_clobbered_p;
 	  
 	  if (! INSN_P (insn))
 	    continue;
@@ -735,17 +742,26 @@ process_bb_node_lives (ira_loop_tree_nod
 
 	  /* If any defined values conflict with the inputs, mark those
 	     defined values as live.  */
+	  some_clobbered_p = false;
 	  for (def_rec = DF_INSN_DEFS (insn); *def_rec; def_rec++)
 	    if (def_conflicts_with_inputs_p (*def_rec))
-	      mark_ref_live (*def_rec);
+	      if (mark_ref_live (*def_rec))
+		some_clobbered_p = true;
 
 	  process_single_reg_class_operands (true, freq);
 	  
 	  /* See which of the defined values we marked as live are dead
 	     before the instruction.  */
-	  for (def_rec = DF_INSN_DEFS (insn); *def_rec; def_rec++)
-	    if (def_conflicts_with_inputs_p (*def_rec))
-	      mark_ref_dead (*def_rec);
+	  if (some_clobbered_p)
+	    {
+	      for (def_rec = DF_INSN_DEFS (insn); *def_rec; def_rec++)
+		if (def_conflicts_with_inputs_p (*def_rec))
+		  mark_ref_dead (*def_rec);
+	      
+	      /* Mark each used value as live again.  */
+	      for (use_rec = DF_INSN_USES (insn); *use_rec; use_rec++)
+		mark_ref_live (*use_rec);
+	    }
 
 	  curr_point++;
 	}