diff mbox series

patch to fix PR82353

Message ID 5794f39e-42ef-66da-e7f5-270ebc54cdf2@redhat.com
State New
Headers show
Series patch to fix PR82353 | expand

Commit Message

Vladimir Makarov Oct. 16, 2017, 8:38 p.m. UTC
This is another version of the patch to fix

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

The patch was successfully bootstrapped on x86-64 with Go and Ada.

Committed as rev. 253796.

Comments

Tom de Vries Dec. 13, 2017, 12:34 p.m. UTC | #1
On 10/16/2017 10:38 PM, Vladimir Makarov wrote:
> This is another version of the patch to fix
> 
>     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353
> 
> The patch was successfully bootstrapped on x86-64 with Go and Ada.
> 
> Committed as rev. 253796.

Hi Vladimir,

AFAIU this bit of the patch makes sure that the flags register show up 
in the bb_livein of the bb in which it's used (and not defined before 
the use), but not in the bb_liveout of the predecessors of that bb.

I wonder if that's a compile-speed optimization, or an oversight.

[ I ran into a similar problem for target gcn here (
   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83327 ) with the
   spill_class hook. I've posted a tentative fix in that PR, which
   piggybacks on this fix, but needed a few extra bits to make sure that
   inter-bb propagation was done:
- a bit at the end of process_bb_lives to detect the liveness change and
   then set live_change_p which make sure the propagation is run.
- a bit in lra_create_live_ranges_1 to unmask the registers we want to
   propagate in all_hard_regs_bitmap.
]

Thanks,
- Tom

> Index: lra-lives.c
> ===================================================================
> --- lra-lives.c	(revision 253685)
> +++ lra-lives.c	(working copy)
> @@ -220,6 +220,9 @@ lra_intersected_live_ranges_p (lra_live_
>     return false;
>   }
>   
> +/* The corresponding bitmaps of BB currently being processed.  */
> +static bitmap bb_killed_pseudos, bb_gen_pseudos;
> +
>   /* The function processing birth of hard register REGNO.  It updates
>      living hard regs, START_LIVING, and conflict hard regs for living
>      pseudos.  Conflict hard regs for the pic pseudo is not updated if
> @@ -243,6 +246,8 @@ make_hard_regno_born (int regno, bool ch
>   	|| i != REGNO (pic_offset_table_rtx))
>   #endif
>         SET_HARD_REG_BIT (lra_reg_info[i].conflict_hard_regs, regno);
> +  if (fixed_regs[regno])
> +    bitmap_set_bit (bb_gen_pseudos, regno);
>   }
>   
>   /* Process the death of hard register REGNO.  This updates
> @@ -255,6 +260,11 @@ make_hard_regno_dead (int regno)
>       return;
>     sparseset_set_bit (start_dying, regno);
>     CLEAR_HARD_REG_BIT (hard_regs_live, regno);
> +  if (fixed_regs[regno])
> +    {
> +      bitmap_clear_bit (bb_gen_pseudos, regno);
> +      bitmap_set_bit (bb_killed_pseudos, regno);
> +    }
>   }
>   
>   /* Mark pseudo REGNO as living at program point POINT, update conflicting
> @@ -299,9 +309,6 @@ mark_pseudo_dead (int regno, int point)
>       }
>   }
>   
> -/* The corresponding bitmaps of BB currently being processed.  */
> -static bitmap bb_killed_pseudos, bb_gen_pseudos;
> -
>   /* Mark register REGNO (pseudo or hard register) in MODE as live at
>      program point POINT.  Update BB_GEN_PSEUDOS.
>      Return TRUE if the liveness tracking sets were modified, or FALSE
>
Vladimir Makarov Dec. 14, 2017, 5:01 p.m. UTC | #2
On 12/13/2017 07:34 AM, Tom de Vries wrote:
> On 10/16/2017 10:38 PM, Vladimir Makarov wrote:
>> This is another version of the patch to fix
>>
>>     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82353
>>
>> The patch was successfully bootstrapped on x86-64 with Go and Ada.
>>
>> Committed as rev. 253796.
>
> Hi Vladimir,
>
> AFAIU this bit of the patch makes sure that the flags register show up 
> in the bb_livein of the bb in which it's used (and not defined before 
> the use), but not in the bb_liveout of the predecessors of that bb.
>
> I wonder if that's a compile-speed optimization, or an oversight.
>
Hi, Tom.  It was just a minimal fix.  I prefer minimal fixes for LRA 
because even for me it is hard to predict in many cases how the patch 
will affect all the targets.  Therefore many LRA patches have a few 
iterations before to be final.

I remember that I had some serious problems in the past when I tried to 
implement fixed hard reg liveness propagation in LRA.  It was long ago 
so we could try it again.  If you send patch you mentioned to gcc 
mailing list, I'll review and approve it.  But we need to be ready to 
revert it if some problems occur again.
diff mbox series

Patch

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 253795)
+++ ChangeLog	(working copy)
@@ -1,3 +1,12 @@ 
+2017-10-16  Vladimir Makarov  <vmakarov@redhat.com>
+
+	PR sanitizer/82353
+	* lra.c (collect_non_operand_hard_regs): Don't ignore operator
+	locations.
+	* lra-lives.c (bb_killed_pseudos, bb_gen_pseudos): Move up.
+	(make_hard_regno_born, make_hard_regno_dead): Update
+	bb_killed_pseudos and bb_gen_pseudos for fixed regs.
+
 2017-10-16  Jeff Law  <law@redhat.com>
 
 	* tree-ssa-dse.c (live_bytes_read): Fix thinko.
Index: lra.c
===================================================================
--- lra.c	(revision 253685)
+++ lra.c	(working copy)
@@ -820,7 +820,8 @@  collect_non_operand_hard_regs (rtx *x, l
   const char *fmt = GET_RTX_FORMAT (code);
 
   for (i = 0; i < data->insn_static_data->n_operands; i++)
-    if (x == data->operand_loc[i])
+    if (! data->insn_static_data->operand[i].is_operator
+	&& x == data->operand_loc[i])
       /* It is an operand loc. Stop here.  */
       return list;
   for (i = 0; i < data->insn_static_data->n_dups; i++)
Index: lra-lives.c
===================================================================
--- lra-lives.c	(revision 253685)
+++ lra-lives.c	(working copy)
@@ -220,6 +220,9 @@  lra_intersected_live_ranges_p (lra_live_
   return false;
 }
 
+/* The corresponding bitmaps of BB currently being processed.  */
+static bitmap bb_killed_pseudos, bb_gen_pseudos;
+
 /* The function processing birth of hard register REGNO.  It updates
    living hard regs, START_LIVING, and conflict hard regs for living
    pseudos.  Conflict hard regs for the pic pseudo is not updated if
@@ -243,6 +246,8 @@  make_hard_regno_born (int regno, bool ch
 	|| i != REGNO (pic_offset_table_rtx))
 #endif
       SET_HARD_REG_BIT (lra_reg_info[i].conflict_hard_regs, regno);
+  if (fixed_regs[regno])
+    bitmap_set_bit (bb_gen_pseudos, regno);
 }
 
 /* Process the death of hard register REGNO.  This updates
@@ -255,6 +260,11 @@  make_hard_regno_dead (int regno)
     return;
   sparseset_set_bit (start_dying, regno);
   CLEAR_HARD_REG_BIT (hard_regs_live, regno);
+  if (fixed_regs[regno])
+    {
+      bitmap_clear_bit (bb_gen_pseudos, regno);
+      bitmap_set_bit (bb_killed_pseudos, regno);
+    }
 }
 
 /* Mark pseudo REGNO as living at program point POINT, update conflicting
@@ -299,9 +309,6 @@  mark_pseudo_dead (int regno, int point)
     }
 }
 
-/* The corresponding bitmaps of BB currently being processed.  */
-static bitmap bb_killed_pseudos, bb_gen_pseudos;
-
 /* Mark register REGNO (pseudo or hard register) in MODE as live at
    program point POINT.  Update BB_GEN_PSEUDOS.
    Return TRUE if the liveness tracking sets were modified, or FALSE