public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
@ 2012-10-13 15:12 Steven Bosscher
  2012-10-13 15:38 ` Vladimir Makarov
  2012-10-13 20:59 ` Eric Botcazou
  0 siblings, 2 replies; 14+ messages in thread
From: Steven Bosscher @ 2012-10-13 15:12 UTC (permalink / raw)
  To: GCC Patches; +Cc: Vladimir Makarov, F. Kenneth Zadeck

[-- Attachment #1: Type: text/plain, Size: 2766 bytes --]

Hello,

This fixes the long-standing enhancement request to use DF_LIVE in
IRA. To quote the first comment in the PR:

IRA should be using the DF-LIVE sets, which are smaller than the DF-LR
sets when they are available (typically at O2 and above). The proper
sets can be conveniently accessed using the df_get_live_[in,out]
functions which use DF-LIVE if it is available and fall back to DF-LR
if it is not.

I thought that DF_LIVE wasn't available at -O1 in IRA, but
interestingly ira.c:ira() adds it to the DF-problems list in that case
already:

$ grep -C3 -n df_live_add ira.c
4160-
4161-  if (optimize == 1)
4162-    {
4163:      df_live_add_problem ();
4164-      df_live_set_all_dirty ();
4165-    }
4166-#ifdef ENABLE_CHECKING

So DF_LIVE is already being computed for IRA. It's just not being used
because the DF_LR_{IN,OUT} macros are used instead of the
df_get_live_{in,out} functions. With my patch, the DF_LIVE sets are
used, which results in marginally faster compile times for my set of
cc1-i files on power64 and on x86_64, as well as smaller code. There's
also a good speedup for the PR54146 test case.

Only reload.c still uses the DF_LR_OUT. The comment in
find_dummy_reload() suggests this is intentional.

I also fond a bug in IRA:

4392-     touch the artificial uses and defs.  */
4393-  df_finish_pass (true);
4394-  if (optimize > 1)
4395:    df_live_add_problem ();
4396-  df_scan_alloc (NULL);
4397-  df_scan_blocks ();
4398-

At optimize>1 the DF_LIVE problem is already always computed. I think
the idea here was to *remove* the DF_LIVE problem at -O1, which is
also part of the attached patch.
(Perhaps we should even simply not add the DF_LIVE problem at -O1, but
that can be done in a follow-up patch if necessary.)

Bootstrapped and tested (with default checking and with release
checking) on powerpc64-unknown-linux-gnu and x86_64-unknown-linux-gnu.
OK for trunk?

Ciao!
Steven

        PR rtl-optimization/38711
        * df.h (df_get_live_out, df_get_live_in): Make static inline functions.
        * df-problems.c (df_get_live_out, df_get_live_in): Moved to df.h.
        * ira-lives.c (process_bb_node_lives): Use df_get_live_out instead of
        DF_LR_OUT.
        * ira-build.c (create_bb_allocnos): Likewise.
        (create_loop_allocnos): Likewise, and use df_get_live_in instead of
        DF_LR_IN.
        * ira-emit.c (generate_edge_moves): Likewise.
        (add_ranges_and_copies): Likewise.
        * ira.c (mark_elimination): Update DF_LR and DF_LIVE.
        (build_insn_chain): Use df_get_live_out instead of DF_LR_OUT.
        (do_reload): Remove the DF_LIVE problem for -O1.
        * ira-color.c (ira_loop_edge_freq): Use df_get_live_out instead of
        DF_LR_OUT, and df_get_live_in instead of DF_LR_IN.

[-- Attachment #2: ira-speedup-3.diff --]
[-- Type: application/octet-stream, Size: 10723 bytes --]

	PR rtl-optimization/38711
	* df.h (df_get_live_out, df_get_live_in): Make static inline functions.
	* df-problems.c (df_get_live_out, df_get_live_in): Moved to df.h.
	* ira-lives.c (process_bb_node_lives): Use df_get_live_out instead of
	DF_LR_OUT.
	* ira-build.c (create_bb_allocnos): Likewise.
	(create_loop_allocnos): Likewise, and use df_get_live_in instead of
	DF_LR_IN.
	* ira-emit.c (generate_edge_moves): Likewise.
	(add_ranges_and_copies): Likewise.
	* ira.c (mark_elimination): Update DF_LR and DF_LIVE.
	(build_insn_chain): Use df_get_live_out instead of DF_LR_OUT.
	(do_reload): Remove the DF_LIVE problem for -O1.
	* ira-color.c (ira_loop_edge_freq): Use df_get_live_out instead of
	DF_LR_OUT, and df_get_live_in instead of DF_LR_IN.

Index: df.h
===================================================================
--- df.h	(revision 192422)
+++ df.h	(working copy)
@@ -951,8 +951,6 @@ extern void debug_df_chain (struct df_link *);
 extern struct df_link *df_chain_create (df_ref, df_ref);
 extern void df_chain_unlink (df_ref);
 extern void df_chain_copy (df_ref, struct df_link *);
-extern bitmap df_get_live_in (basic_block);
-extern bitmap df_get_live_out (basic_block);
 extern void df_grow_bb_info (struct dataflow *);
 extern void df_chain_dump (struct df_link *, FILE *);
 extern void df_print_bb_index (basic_block bb, FILE *file);
@@ -1023,7 +1021,10 @@ extern void df_compute_regs_ever_live (bool);
 extern bool df_read_modify_subreg_p (rtx);
 extern void df_scan_verify (void);
 
-/* Get basic block info.  */
+\f
+/*----------------------------------------------------------------------------
+   Public functions access functions for the dataflow problems.
+----------------------------------------------------------------------------*/
 
 static inline struct df_scan_bb_info *
 df_scan_get_bb_info (unsigned int index)
@@ -1079,6 +1080,39 @@ df_word_lr_get_bb_info (unsigned int index)
     return NULL;
 }
 
+/* Get the live at out set for BB no matter what problem happens to be
+   defined.  This function is used by the register allocators who
+   choose different dataflow problems depending on the optimization
+   level.  */
+
+static inline bitmap
+df_get_live_out (basic_block bb)
+{
+  gcc_checking_assert (df_lr);
+
+  if (df_live)
+    return DF_LIVE_OUT (bb);
+  else
+    return DF_LR_OUT (bb);
+}
+
+/* Get the live at in set for BB no matter what problem happens to be
+   defined.  This function is used by the register allocators who
+   choose different dataflow problems depending on the optimization
+   level.  */
+
+static inline bitmap
+df_get_live_in (basic_block bb)
+{
+  gcc_checking_assert (df_lr);
+
+  if (df_live)
+    return DF_LIVE_IN (bb);
+  else
+    return DF_LR_IN (bb);
+}
+
+/* Get basic block info.  */
 /* Get the artificial defs for a basic block.  */
 
 static inline df_ref *
Index: df-problems.c
===================================================================
--- df-problems.c	(revision 192422)
+++ df-problems.c	(working copy)
@@ -57,43 +57,7 @@ along with GCC; see the file COPYING3.  If not see
 static bitmap_head seen_in_block;
 static bitmap_head seen_in_insn;
 
-\f
 /*----------------------------------------------------------------------------
-   Public functions access functions for the dataflow problems.
-----------------------------------------------------------------------------*/
-/* Get the live at out set for BB no matter what problem happens to be
-   defined.  This function is used by the register allocators who
-   choose different dataflow problems depending on the optimization
-   level.  */
-
-bitmap
-df_get_live_out (basic_block bb)
-{
-  gcc_assert (df_lr);
-
-  if (df_live)
-    return DF_LIVE_OUT (bb);
-  else
-    return DF_LR_OUT (bb);
-}
-
-/* Get the live at in set for BB no matter what problem happens to be
-   defined.  This function is used by the register allocators who
-   choose different dataflow problems depending on the optimization
-   level.  */
-
-bitmap
-df_get_live_in (basic_block bb)
-{
-  gcc_assert (df_lr);
-
-  if (df_live)
-    return DF_LIVE_IN (bb);
-  else
-    return DF_LR_IN (bb);
-}
-
-/*----------------------------------------------------------------------------
    Utility functions.
 ----------------------------------------------------------------------------*/
 
Index: ira-lives.c
===================================================================
--- ira-lives.c	(revision 192422)
+++ ira-lives.c	(working copy)
@@ -1148,7 +1148,7 @@ process_bb_node_lives (ira_loop_tree_node_t loop_t
 	  high_pressure_start_point[ira_pressure_classes[i]] = -1;
 	}
       curr_bb_node = loop_tree_node;
-      reg_live_out = DF_LR_OUT (bb);
+      reg_live_out = df_get_live_out (bb);
       sparseset_clear (objects_live);
       REG_SET_TO_HARD_REG_SET (hard_regs_live, reg_live_out);
       AND_COMPL_HARD_REG_SET (hard_regs_live, eliminable_regset);
Index: ira-build.c
===================================================================
--- ira-build.c	(revision 192422)
+++ ira-build.c	(working copy)
@@ -1715,7 +1715,7 @@ create_bb_allocnos (ira_loop_tree_node_t bb_node)
       create_insn_allocnos (PATTERN (insn), false);
   /* It might be a allocno living through from one subloop to
      another.  */
-  EXECUTE_IF_SET_IN_REG_SET (DF_LR_IN (bb), FIRST_PSEUDO_REGISTER, i, bi)
+  EXECUTE_IF_SET_IN_REG_SET (df_get_live_in (bb), FIRST_PSEUDO_REGISTER, i, bi)
     if (ira_curr_regno_allocno_map[i] == NULL)
       ira_create_allocno (i, false, ira_curr_loop_tree_node);
 }
@@ -1731,9 +1731,9 @@ create_loop_allocnos (edge e)
   bitmap_iterator bi;
   ira_loop_tree_node_t parent;
 
-  live_in_regs = DF_LR_IN (e->dest);
+  live_in_regs = df_get_live_in (e->dest);
   border_allocnos = ira_curr_loop_tree_node->border_allocnos;
-  EXECUTE_IF_SET_IN_REG_SET (DF_LR_OUT (e->src),
+  EXECUTE_IF_SET_IN_REG_SET (df_get_live_out (e->src),
 			     FIRST_PSEUDO_REGISTER, i, bi)
     if (bitmap_bit_p (live_in_regs, i))
       {
Index: ira-emit.c
===================================================================
--- ira-emit.c	(revision 192422)
+++ ira-emit.c	(working copy)
@@ -495,6 +495,7 @@ generate_edge_moves (edge e)
   bitmap_iterator bi;
   ira_allocno_t src_allocno, dest_allocno, *src_map, *dest_map;
   move_t move;
+  bitmap regs_live_in_dest, regs_live_out_src;
 
   src_loop_node = IRA_BB_NODE (e->src)->parent;
   dest_loop_node = IRA_BB_NODE (e->dest)->parent;
@@ -503,9 +504,11 @@ generate_edge_moves (edge e)
     return;
   src_map = src_loop_node->regno_allocno_map;
   dest_map = dest_loop_node->regno_allocno_map;
-  EXECUTE_IF_SET_IN_REG_SET (DF_LR_IN (e->dest),
+  regs_live_in_dest = df_get_live_in (e->dest);
+  regs_live_out_src = df_get_live_out (e->src);
+  EXECUTE_IF_SET_IN_REG_SET (regs_live_in_dest,
 			     FIRST_PSEUDO_REGISTER, regno, bi)
-    if (bitmap_bit_p (DF_LR_OUT (e->src), regno))
+    if (bitmap_bit_p (regs_live_out_src, regno))
       {
 	src_allocno = src_map[regno];
 	dest_allocno = dest_map[regno];
@@ -1206,15 +1209,16 @@ add_ranges_and_copies (void)
 	 destination block) to use for searching allocnos by their
 	 regnos because of subsequent IR flattening.  */
       node = IRA_BB_NODE (bb)->parent;
-      bitmap_copy (live_through, DF_LR_IN (bb));
+      bitmap_copy (live_through, df_get_live_in (bb));
       add_range_and_copies_from_move_list
 	(at_bb_start[bb->index], node, live_through, REG_FREQ_FROM_BB (bb));
-      bitmap_copy (live_through, DF_LR_OUT (bb));
+      bitmap_copy (live_through, df_get_live_out (bb));
       add_range_and_copies_from_move_list
 	(at_bb_end[bb->index], node, live_through, REG_FREQ_FROM_BB (bb));
       FOR_EACH_EDGE (e, ei, bb->succs)
 	{
-	  bitmap_and (live_through, DF_LR_IN (e->dest), DF_LR_OUT (bb));
+	  bitmap_and (live_through,
+		      df_get_live_in (e->dest), df_get_live_out (bb));
 	  add_range_and_copies_from_move_list
 	    ((move_t) e->aux, node, live_through,
 	     REG_FREQ_FROM_EDGE_FREQ (EDGE_FREQUENCY (e)));
Index: ira.c
===================================================================
--- ira.c	(revision 192423)
+++ ira.c	(working copy)
@@ -2337,17 +2337,24 @@ void
 mark_elimination (int from, int to)
 {
   basic_block bb;
+  bitmap r;
 
   FOR_EACH_BB (bb)
     {
-      /* We don't use LIVE info in IRA.  */
-      bitmap r = DF_LR_IN (bb);
-
-      if (REGNO_REG_SET_P (r, from))
+      r = DF_LR_IN (bb);
+      if (bitmap_bit_p (r, from))
 	{
-	  CLEAR_REGNO_REG_SET (r, from);
-	  SET_REGNO_REG_SET (r, to);
+	  bitmap_clear_bit (r, from);
+	  bitmap_set_bit (r, to);
 	}
+      if (! df_live)
+        continue;
+      r = DF_LIVE_IN (bb);
+      if (bitmap_bit_p (r, from))
+	{
+	  bitmap_clear_bit (r, from);
+	  bitmap_set_bit (r, to);
+	}
     }
 }
 
@@ -3319,14 +3326,14 @@ build_insn_chain (void)
       CLEAR_REG_SET (live_relevant_regs);
       bitmap_clear (live_subregs_used);
 
-      EXECUTE_IF_SET_IN_BITMAP (DF_LR_OUT (bb), 0, i, bi)
+      EXECUTE_IF_SET_IN_BITMAP (df_get_live_out (bb), 0, i, bi)
 	{
 	  if (i >= FIRST_PSEUDO_REGISTER)
 	    break;
 	  bitmap_set_bit (live_relevant_regs, i);
 	}
 
-      EXECUTE_IF_SET_IN_BITMAP (DF_LR_OUT (bb),
+      EXECUTE_IF_SET_IN_BITMAP (df_get_live_out (bb),
 				FIRST_PSEUDO_REGISTER, i, bi)
 	{
 	  if (pseudo_for_reload_consideration_p (i))
@@ -4397,8 +4404,8 @@ do_reload (void)
      df_rescan_all_insns is not going to help here because it does not
      touch the artificial uses and defs.  */
   df_finish_pass (true);
-  if (optimize > 1)
-    df_live_add_problem ();
+  if (optimize == 1)
+    df_remove_problem (df_live);
   df_scan_alloc (NULL);
   df_scan_blocks ();
 
Index: ira-color.c
===================================================================
--- ira-color.c	(revision 192422)
+++ ira-color.c	(working copy)
@@ -2014,8 +2014,8 @@ ira_loop_edge_freq (ira_loop_tree_node_t loop_node
       FOR_EACH_EDGE (e, ei, loop_node->loop->header->preds)
 	if (e->src != loop_node->loop->latch
 	    && (regno < 0
-		|| (bitmap_bit_p (DF_LR_OUT (e->src), regno)
-		    && bitmap_bit_p (DF_LR_IN (e->dest), regno))))
+		|| (bitmap_bit_p (df_get_live_out (e->src), regno)
+		    && bitmap_bit_p (df_get_live_in (e->dest), regno))))
 	  freq += EDGE_FREQUENCY (e);
     }
   else
@@ -2023,8 +2023,8 @@ ira_loop_edge_freq (ira_loop_tree_node_t loop_node
       edges = get_loop_exit_edges (loop_node->loop);
       FOR_EACH_VEC_ELT (edge, edges, i, e)
 	if (regno < 0
-	    || (bitmap_bit_p (DF_LR_OUT (e->src), regno)
-		&& bitmap_bit_p (DF_LR_IN (e->dest), regno)))
+	    || (bitmap_bit_p (df_get_live_out (e->src), regno)
+		&& bitmap_bit_p (df_get_live_in (e->dest), regno)))
 	  freq += EDGE_FREQUENCY (e);
       VEC_free (edge, heap, edges);
     }

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-13 15:12 [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher) Steven Bosscher
@ 2012-10-13 15:38 ` Vladimir Makarov
  2012-10-13 15:51   ` Steven Bosscher
  2012-10-13 20:59 ` Eric Botcazou
  1 sibling, 1 reply; 14+ messages in thread
From: Vladimir Makarov @ 2012-10-13 15:38 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: GCC Patches, F. Kenneth Zadeck

On 12-10-13 9:40 AM, Steven Bosscher wrote:
> Hello,
>
> This fixes the long-standing enhancement request to use DF_LIVE in
> IRA. To quote the first comment in the PR:
>
> IRA should be using the DF-LIVE sets, which are smaller than the DF-LR
> sets when they are available (typically at O2 and above). The proper
> sets can be conveniently accessed using the df_get_live_[in,out]
> functions which use DF-LIVE if it is available and fall back to DF-LR
> if it is not.
>
> I thought that DF_LIVE wasn't available at -O1 in IRA, but
> interestingly ira.c:ira() adds it to the DF-problems list in that case
> already:
>
> $ grep -C3 -n df_live_add ira.c
> 4160-
> 4161-  if (optimize == 1)
> 4162-    {
> 4163:      df_live_add_problem ();
> 4164-      df_live_set_all_dirty ();
> 4165-    }
> 4166-#ifdef ENABLE_CHECKING
>
> So DF_LIVE is already being computed for IRA. It's just not being used
> because the DF_LR_{IN,OUT} macros are used instead of the
> df_get_live_{in,out} functions. With my patch, the DF_LIVE sets are
> used, which results in marginally faster compile times for my set of
> cc1-i files on power64 and on x86_64, as well as smaller code. There's
> also a good speedup for the PR54146 test case.
>
> Only reload.c still uses the DF_LR_OUT. The comment in
> find_dummy_reload() suggests this is intentional.
>
> I also fond a bug in IRA:
>
> 4392-     touch the artificial uses and defs.  */
> 4393-  df_finish_pass (true);
> 4394-  if (optimize > 1)
> 4395:    df_live_add_problem ();
> 4396-  df_scan_alloc (NULL);
> 4397-  df_scan_blocks ();
> 4398-
>
> At optimize>1 the DF_LIVE problem is already always computed. I think
> the idea here was to *remove* the DF_LIVE problem at -O1, which is
> also part of the attached patch.
> (Perhaps we should even simply not add the DF_LIVE problem at -O1, but
> that can be done in a follow-up patch if necessary.)
>
> Bootstrapped and tested (with default checking and with release
> checking) on powerpc64-unknown-linux-gnu and x86_64-unknown-linux-gnu.
> OK for trunk?
>
>
LIVE is not used on purpose.  I added LIVE usage to the old RA about 10 
years ago (it was before DF-infrastructure).  Everything was ok until, 
somebody reported compiler crash on semantically wrong program 
(something with usage of uninitialized variable).  After that I removed 
this code.

Probably, that is also a reason why reload uses LR not LIVE.  So I'd 
like to wait a few weeks.  When I have more time, I am going to 
reproduce this test suite and look how IRA behaves on it.

Thanks, Steven.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-13 15:38 ` Vladimir Makarov
@ 2012-10-13 15:51   ` Steven Bosscher
  2012-10-13 21:41     ` Vladimir Makarov
  0 siblings, 1 reply; 14+ messages in thread
From: Steven Bosscher @ 2012-10-13 15:51 UTC (permalink / raw)
  To: Vladimir Makarov; +Cc: GCC Patches, F. Kenneth Zadeck

On Sat, Oct 13, 2012 at 5:12 PM, Vladimir Makarov wrote:
> On 12-10-13 9:40 AM, Steven Bosscher wrote:
>>
>> Hello,
>>
>> This fixes the long-standing enhancement request to use DF_LIVE in
>> IRA. To quote the first comment in the PR:
>>
>> IRA should be using the DF-LIVE sets, which are smaller than the DF-LR
>> sets when they are available (typically at O2 and above). The proper
>> sets can be conveniently accessed using the df_get_live_[in,out]
>> functions which use DF-LIVE if it is available and fall back to DF-LR
>> if it is not.
>>
>> I thought that DF_LIVE wasn't available at -O1 in IRA, but
>> interestingly ira.c:ira() adds it to the DF-problems list in that case
>> already:
>>
>> $ grep -C3 -n df_live_add ira.c
>> 4160-
>> 4161-  if (optimize == 1)
>> 4162-    {
>> 4163:      df_live_add_problem ();
>> 4164-      df_live_set_all_dirty ();
>> 4165-    }
>> 4166-#ifdef ENABLE_CHECKING
>>
>> So DF_LIVE is already being computed for IRA. It's just not being used
>> because the DF_LR_{IN,OUT} macros are used instead of the
>> df_get_live_{in,out} functions. With my patch, the DF_LIVE sets are
>> used, which results in marginally faster compile times for my set of
>> cc1-i files on power64 and on x86_64, as well as smaller code. There's
>> also a good speedup for the PR54146 test case.
>>
>> Only reload.c still uses the DF_LR_OUT. The comment in
>> find_dummy_reload() suggests this is intentional.
>>
>> I also fond a bug in IRA:
>>
>> 4392-     touch the artificial uses and defs.  */
>> 4393-  df_finish_pass (true);
>> 4394-  if (optimize > 1)
>> 4395:    df_live_add_problem ();
>> 4396-  df_scan_alloc (NULL);
>> 4397-  df_scan_blocks ();
>> 4398-
>>
>> At optimize>1 the DF_LIVE problem is already always computed. I think
>> the idea here was to *remove* the DF_LIVE problem at -O1, which is
>> also part of the attached patch.
>> (Perhaps we should even simply not add the DF_LIVE problem at -O1, but
>> that can be done in a follow-up patch if necessary.)
>>
>> Bootstrapped and tested (with default checking and with release
>> checking) on powerpc64-unknown-linux-gnu and x86_64-unknown-linux-gnu.
>> OK for trunk?
>>
>>
> LIVE is not used on purpose.  I added LIVE usage to the old RA about 10
> years ago (it was before DF-infrastructure).  Everything was ok until,
> somebody reported compiler crash on semantically wrong program (something
> with usage of uninitialized variable).  After that I removed this code.

GCC was a completely different compiler 10 years ago. Handling of
uninitialized variables is radically different since tree-ssa, and
also the resource allocation side of the compiler has essentially been
rewritten (by you :-). Whatever broke in the compiler 10 years ago may
not be relevant anymore today.

I can't find your work, or the reported problem, in the gcc mailing
list archives. Did you add a test case for the problem, and if so,
where can I find it?


> Probably, that is also a reason why reload uses LR not LIVE.

The reason why reload uses DF_LR is PR20973, as documented in the code.
(And FWIW, that bug doesn't reproduce even with GCC 4.0 if you add the
transformations of pass_initialize_regs to it, that pass exists to
avoid uninitialized regs hazards by making them initialized.)


>  So I'd like to
> wait a few weeks.  When I have more time, I am going to reproduce this test
> suite and look how IRA behaves on it.

I don't think that's giving this patch a fair chance. In a few week's
time, stage1 will be over. You may not have time, but that doesn't
mean other developers should wait. If you have a test case that fails
with my patch, I'll gladly spend my own time on it to see if it can be
fixed.

My patch was properly tested on two primary targets and passes all
tests in the test suite, it improves the compiler now, and it
addresses a PR that you've basically ignored for 4 years. I kindly ask
you to reconsider your position :-)

Ciao!
Steven

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-13 15:12 [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher) Steven Bosscher
  2012-10-13 15:38 ` Vladimir Makarov
@ 2012-10-13 20:59 ` Eric Botcazou
  2012-10-13 21:08   ` Steven Bosscher
  1 sibling, 1 reply; 14+ messages in thread
From: Eric Botcazou @ 2012-10-13 20:59 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: gcc-patches, Vladimir Makarov, F. Kenneth Zadeck

> IRA should be using the DF-LIVE sets, which are smaller than the DF-LR
> sets when they are available (typically at O2 and above). The proper
> sets can be conveniently accessed using the df_get_live_[in,out]
> functions which use DF-LIVE if it is available and fall back to DF-LR
> if it is not.
> [...]
> At optimize>1 the DF_LIVE problem is already always computed. I think
> the idea here was to *remove* the DF_LIVE problem at -O1, which is
> also part of the attached patch.
> (Perhaps we should even simply not add the DF_LIVE problem at -O1, but
> that can be done in a follow-up patch if necessary.)

So, in the end, does the patch enable DF_LIVE at -O1 or not?  There seems to 
be a contradiction between the subject and the body of the message.  If yes, 
perhaps an acceptable compromise would be to keep things unchanged at -O1.

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-13 20:59 ` Eric Botcazou
@ 2012-10-13 21:08   ` Steven Bosscher
  2012-10-13 21:15     ` Eric Botcazou
  0 siblings, 1 reply; 14+ messages in thread
From: Steven Bosscher @ 2012-10-13 21:08 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Vladimir Makarov, F. Kenneth Zadeck

On Sat, Oct 13, 2012 at 10:38 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> So, in the end, does the patch enable DF_LIVE at -O1 or not?  There seems to
> be a contradiction between the subject and the body of the message.  If yes,
> perhaps an acceptable compromise would be to keep things unchanged at -O1.

Eh, right. When I wrote the e-mail, I was still under the impression
that IRA only has DF_LIVE available at -O2 and higher, and I only
discovered that DF_LIVE is added if optimize==1 just before submitting
the patch. So my patch as submitted should have said "... (for -O1 and
higher)".

I think it would be a good idea to keep things unchanged at -O1. For
that, the patch needs a few minor modifications (remove calls to
df_live_add_problem and make some code to update DF_LIVE_{IN,OUT}
conditional). I can prepare an updated patch for that, if you think
that's best.

Ciao!
Steven

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-13 21:08   ` Steven Bosscher
@ 2012-10-13 21:15     ` Eric Botcazou
  2012-10-13 21:52       ` Steven Bosscher
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Botcazou @ 2012-10-13 21:15 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: gcc-patches, Vladimir Makarov, F. Kenneth Zadeck

> I think it would be a good idea to keep things unchanged at -O1. For
> that, the patch needs a few minor modifications (remove calls to
> df_live_add_problem and make some code to update DF_LIVE_{IN,OUT}
> conditional). I can prepare an updated patch for that, if you think
> that's best.

That was just a proposal to be put on the table. :-)  Others, especially Vlad 
of course, have the final say.  And I somewhat sympathize with the cautious 
approach here, because we were forced to downgrade from DF_LIVE to DF_LR for 
resource.c at some point (but of course it's resource.c so...).

-- 
Eric Botcazou

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-13 15:51   ` Steven Bosscher
@ 2012-10-13 21:41     ` Vladimir Makarov
  2012-10-14 11:42       ` Steven Bosscher
  2012-10-27 23:36       ` Steven Bosscher
  0 siblings, 2 replies; 14+ messages in thread
From: Vladimir Makarov @ 2012-10-13 21:41 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: GCC Patches, F. Kenneth Zadeck

On 12-10-13 11:37 AM, Steven Bosscher wrote:
> On Sat, Oct 13, 2012 at 5:12 PM, Vladimir Makarov wrote:
>> On 12-10-13 9:40 AM, Steven Bosscher wrote:
>>> Hello,
>>>
>>> This fixes the long-standing enhancement request to use DF_LIVE in
>>> IRA. To quote the first comment in the PR:
>>>
>>> IRA should be using the DF-LIVE sets, which are smaller than the DF-LR
>>> sets when they are available (typically at O2 and above). The proper
>>> sets can be conveniently accessed using the df_get_live_[in,out]
>>> functions which use DF-LIVE if it is available and fall back to DF-LR
>>> if it is not.
>>>
>>> I thought that DF_LIVE wasn't available at -O1 in IRA, but
>>> interestingly ira.c:ira() adds it to the DF-problems list in that case
>>> already:
>>>
>>> $ grep -C3 -n df_live_add ira.c
>>> 4160-
>>> 4161-  if (optimize == 1)
>>> 4162-    {
>>> 4163:      df_live_add_problem ();
>>> 4164-      df_live_set_all_dirty ();
>>> 4165-    }
>>> 4166-#ifdef ENABLE_CHECKING
>>>
>>> So DF_LIVE is already being computed for IRA. It's just not being used
>>> because the DF_LR_{IN,OUT} macros are used instead of the
>>> df_get_live_{in,out} functions. With my patch, the DF_LIVE sets are
>>> used, which results in marginally faster compile times for my set of
>>> cc1-i files on power64 and on x86_64, as well as smaller code. There's
>>> also a good speedup for the PR54146 test case.
>>>
>>> Only reload.c still uses the DF_LR_OUT. The comment in
>>> find_dummy_reload() suggests this is intentional.
>>>
>>> I also fond a bug in IRA:
>>>
>>> 4392-     touch the artificial uses and defs.  */
>>> 4393-  df_finish_pass (true);
>>> 4394-  if (optimize > 1)
>>> 4395:    df_live_add_problem ();
>>> 4396-  df_scan_alloc (NULL);
>>> 4397-  df_scan_blocks ();
>>> 4398-
>>>
>>> At optimize>1 the DF_LIVE problem is already always computed. I think
>>> the idea here was to *remove* the DF_LIVE problem at -O1, which is
>>> also part of the attached patch.
>>> (Perhaps we should even simply not add the DF_LIVE problem at -O1, but
>>> that can be done in a follow-up patch if necessary.)
>>>
>>> Bootstrapped and tested (with default checking and with release
>>> checking) on powerpc64-unknown-linux-gnu and x86_64-unknown-linux-gnu.
>>> OK for trunk?
>>>
>>>
>> LIVE is not used on purpose.  I added LIVE usage to the old RA about 10
>> years ago (it was before DF-infrastructure).  Everything was ok until,
>> somebody reported compiler crash on semantically wrong program (something
>> with usage of uninitialized variable).  After that I removed this code.
> GCC was a completely different compiler 10 years ago. Handling of
> uninitialized variables is radically different since tree-ssa, and
> also the resource allocation side of the compiler has essentially been
> rewritten (by you :-). Whatever broke in the compiler 10 years ago may
> not be relevant anymore today.
>
> I can't find your work, or the reported problem, in the gcc mailing
> list archives. Did you add a test case for the problem, and if so,
> where can I find it?
>
>
>> Probably, that is also a reason why reload uses LR not LIVE.
> The reason why reload uses DF_LR is PR20973, as documented in the code.
> (And FWIW, that bug doesn't reproduce even with GCC 4.0 if you add the
> transformations of pass_initialize_regs to it, that pass exists to
> avoid uninitialized regs hazards by making them initialized.)
>
>
>>   So I'd like to
>> wait a few weeks.  When I have more time, I am going to reproduce this test
>> suite and look how IRA behaves on it.
> I don't think that's giving this patch a fair chance. In a few week's
> time, stage1 will be over. You may not have time, but that doesn't
> mean other developers should wait. If you have a test case that fails
> with my patch, I'll gladly spend my own time on it to see if it can be
> fixed.
>
> My patch was properly tested on two primary targets and passes all
> tests in the test suite, it improves the compiler now, and it
> addresses a PR that you've basically ignored for 4 years. I kindly ask
> you to reconsider your position :-)
>
I guess you are right.  A lot of things are changed.  DF is also 
different from what I used.  Other passes also uses LIVE sets.

Ok for the idea.  If we have a problem later, we could fix it.  I'll 
look at the next version of the patch when you send it to give your the 
final approval.

Thanks for your attention to IRA.

By the way, I found the reported problem:

http://gcc.gnu.org/ml/gcc-patches/2004-06/msg00533.html

But it occurred on s390.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-13 21:15     ` Eric Botcazou
@ 2012-10-13 21:52       ` Steven Bosscher
  0 siblings, 0 replies; 14+ messages in thread
From: Steven Bosscher @ 2012-10-13 21:52 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, Vladimir Makarov, F. Kenneth Zadeck

On Sat, Oct 13, 2012 at 11:05 PM, Eric Botcazou wrote:
>> I think it would be a good idea to keep things unchanged at -O1. For
>> that, the patch needs a few minor modifications (remove calls to
>> df_live_add_problem and make some code to update DF_LIVE_{IN,OUT}
>> conditional). I can prepare an updated patch for that, if you think
>> that's best.
>
> That was just a proposal to be put on the table. :-)  Others, especially Vlad
> of course, have the final say.  And I somewhat sympathize with the cautious
> approach here, because we were forced to downgrade from DF_LIVE to DF_LR for
> resource.c at some point (but of course it's resource.c so...).

Right, that was for PR40710
(http://gcc.gnu.org/ml/gcc/2009-07/msg00241.html) because resource.c
computes its own idea of liveness and it uses a similar definition as
the DF_LR problem. Using incompatible views of liveness is not a good
idea...

For IRA it doesn't matter whether you use DF_LR or DF_LIVE, as long as
you use one or the other consistently.

Ciao!
Steven

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-13 21:41     ` Vladimir Makarov
@ 2012-10-14 11:42       ` Steven Bosscher
  2012-10-14 17:37         ` Vladimir Makarov
  2012-10-27 23:36       ` Steven Bosscher
  1 sibling, 1 reply; 14+ messages in thread
From: Steven Bosscher @ 2012-10-14 11:42 UTC (permalink / raw)
  To: Vladimir Makarov; +Cc: GCC Patches, F. Kenneth Zadeck

[-- Attachment #1: Type: text/plain, Size: 349 bytes --]

On Sat, Oct 13, 2012 at 11:12 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
> Ok for the idea.  If we have a problem later, we could fix it.  I'll look at
> the next version of the patch when you send it to give your the final
> approval.

Great, thanks!

Here is the updated patch, tested in the same way as the previous version.

Ciao!
Steven

[-- Attachment #2: ira-speedup-3.diff --]
[-- Type: application/octet-stream, Size: 11436 bytes --]

	PR rtl-optimization/38711
	* df.h (df_get_live_out, df_get_live_in): Make static inline functions.
	* df-problems.c (df_get_live_out, df_get_live_in): Moved to df.h.
	* ira-lives.c (process_bb_node_lives): Use df_get_live_out instead of
	DF_LR_OUT.
	* ira-build.c (create_bb_allocnos): Likewise.
	(create_loop_allocnos): Likewise, and use df_get_live_in instead of
	DF_LR_IN.
	* ira-emit.c (generate_edge_moves): Likewise.
	(add_ranges_and_copies): Likewise.
	* ira-color.c (ira_loop_edge_freq): Use df_get_live_out instead of
	DF_LR_OUT, and df_get_live_in instead of DF_LR_IN.
	* ira.c (mark_elimination): Update DF_LR and DF_LIVE.
	(build_insn_chain): Use df_get_live_out instead of DF_LR_OUT.
	(do_reload): Remove the DF_LIVE problem for -O1.

Index: df.h
===================================================================
--- df.h	(revision 192426)
+++ df.h	(working copy)
@@ -951,8 +951,6 @@ extern void debug_df_chain (struct df_li
 extern struct df_link *df_chain_create (df_ref, df_ref);
 extern void df_chain_unlink (df_ref);
 extern void df_chain_copy (df_ref, struct df_link *);
-extern bitmap df_get_live_in (basic_block);
-extern bitmap df_get_live_out (basic_block);
 extern void df_grow_bb_info (struct dataflow *);
 extern void df_chain_dump (struct df_link *, FILE *);
 extern void df_print_bb_index (basic_block bb, FILE *file);
@@ -1023,7 +1021,10 @@ extern void df_compute_regs_ever_live (b
 extern bool df_read_modify_subreg_p (rtx);
 extern void df_scan_verify (void);
 
-/* Get basic block info.  */
+\f
+/*----------------------------------------------------------------------------
+   Public functions access functions for the dataflow problems.
+----------------------------------------------------------------------------*/
 
 static inline struct df_scan_bb_info *
 df_scan_get_bb_info (unsigned int index)
@@ -1079,6 +1080,39 @@ df_word_lr_get_bb_info (unsigned int ind
     return NULL;
 }
 
+/* Get the live at out set for BB no matter what problem happens to be
+   defined.  This function is used by the register allocators who
+   choose different dataflow problems depending on the optimization
+   level.  */
+
+static inline bitmap
+df_get_live_out (basic_block bb)
+{
+  gcc_checking_assert (df_lr);
+
+  if (df_live)
+    return DF_LIVE_OUT (bb);
+  else
+    return DF_LR_OUT (bb);
+}
+
+/* Get the live at in set for BB no matter what problem happens to be
+   defined.  This function is used by the register allocators who
+   choose different dataflow problems depending on the optimization
+   level.  */
+
+static inline bitmap
+df_get_live_in (basic_block bb)
+{
+  gcc_checking_assert (df_lr);
+
+  if (df_live)
+    return DF_LIVE_IN (bb);
+  else
+    return DF_LR_IN (bb);
+}
+
+/* Get basic block info.  */
 /* Get the artificial defs for a basic block.  */
 
 static inline df_ref *
Index: df-problems.c
===================================================================
--- df-problems.c	(revision 192426)
+++ df-problems.c	(working copy)
@@ -57,42 +57,6 @@ along with GCC; see the file COPYING3.  
 static bitmap_head seen_in_block;
 static bitmap_head seen_in_insn;
 
-\f
-/*----------------------------------------------------------------------------
-   Public functions access functions for the dataflow problems.
-----------------------------------------------------------------------------*/
-/* Get the live at out set for BB no matter what problem happens to be
-   defined.  This function is used by the register allocators who
-   choose different dataflow problems depending on the optimization
-   level.  */
-
-bitmap
-df_get_live_out (basic_block bb)
-{
-  gcc_assert (df_lr);
-
-  if (df_live)
-    return DF_LIVE_OUT (bb);
-  else
-    return DF_LR_OUT (bb);
-}
-
-/* Get the live at in set for BB no matter what problem happens to be
-   defined.  This function is used by the register allocators who
-   choose different dataflow problems depending on the optimization
-   level.  */
-
-bitmap
-df_get_live_in (basic_block bb)
-{
-  gcc_assert (df_lr);
-
-  if (df_live)
-    return DF_LIVE_IN (bb);
-  else
-    return DF_LR_IN (bb);
-}
-
 /*----------------------------------------------------------------------------
    Utility functions.
 ----------------------------------------------------------------------------*/
Index: ira-lives.c
===================================================================
--- ira-lives.c	(revision 192426)
+++ ira-lives.c	(working copy)
@@ -1148,7 +1148,7 @@ process_bb_node_lives (ira_loop_tree_nod
 	  high_pressure_start_point[ira_pressure_classes[i]] = -1;
 	}
       curr_bb_node = loop_tree_node;
-      reg_live_out = DF_LR_OUT (bb);
+      reg_live_out = df_get_live_out (bb);
       sparseset_clear (objects_live);
       REG_SET_TO_HARD_REG_SET (hard_regs_live, reg_live_out);
       AND_COMPL_HARD_REG_SET (hard_regs_live, eliminable_regset);
Index: ira-build.c
===================================================================
--- ira-build.c	(revision 192426)
+++ ira-build.c	(working copy)
@@ -1715,7 +1715,7 @@ create_bb_allocnos (ira_loop_tree_node_t
       create_insn_allocnos (PATTERN (insn), false);
   /* It might be a allocno living through from one subloop to
      another.  */
-  EXECUTE_IF_SET_IN_REG_SET (DF_LR_IN (bb), FIRST_PSEUDO_REGISTER, i, bi)
+  EXECUTE_IF_SET_IN_REG_SET (df_get_live_in (bb), FIRST_PSEUDO_REGISTER, i, bi)
     if (ira_curr_regno_allocno_map[i] == NULL)
       ira_create_allocno (i, false, ira_curr_loop_tree_node);
 }
@@ -1731,9 +1731,9 @@ create_loop_allocnos (edge e)
   bitmap_iterator bi;
   ira_loop_tree_node_t parent;
 
-  live_in_regs = DF_LR_IN (e->dest);
+  live_in_regs = df_get_live_in (e->dest);
   border_allocnos = ira_curr_loop_tree_node->border_allocnos;
-  EXECUTE_IF_SET_IN_REG_SET (DF_LR_OUT (e->src),
+  EXECUTE_IF_SET_IN_REG_SET (df_get_live_out (e->src),
 			     FIRST_PSEUDO_REGISTER, i, bi)
     if (bitmap_bit_p (live_in_regs, i))
       {
Index: ira-emit.c
===================================================================
--- ira-emit.c	(revision 192426)
+++ ira-emit.c	(working copy)
@@ -495,6 +495,7 @@ generate_edge_moves (edge e)
   bitmap_iterator bi;
   ira_allocno_t src_allocno, dest_allocno, *src_map, *dest_map;
   move_t move;
+  bitmap regs_live_in_dest, regs_live_out_src;
 
   src_loop_node = IRA_BB_NODE (e->src)->parent;
   dest_loop_node = IRA_BB_NODE (e->dest)->parent;
@@ -503,9 +504,11 @@ generate_edge_moves (edge e)
     return;
   src_map = src_loop_node->regno_allocno_map;
   dest_map = dest_loop_node->regno_allocno_map;
-  EXECUTE_IF_SET_IN_REG_SET (DF_LR_IN (e->dest),
+  regs_live_in_dest = df_get_live_in (e->dest);
+  regs_live_out_src = df_get_live_out (e->src);
+  EXECUTE_IF_SET_IN_REG_SET (regs_live_in_dest,
 			     FIRST_PSEUDO_REGISTER, regno, bi)
-    if (bitmap_bit_p (DF_LR_OUT (e->src), regno))
+    if (bitmap_bit_p (regs_live_out_src, regno))
       {
 	src_allocno = src_map[regno];
 	dest_allocno = dest_map[regno];
@@ -1206,15 +1209,16 @@ add_ranges_and_copies (void)
 	 destination block) to use for searching allocnos by their
 	 regnos because of subsequent IR flattening.  */
       node = IRA_BB_NODE (bb)->parent;
-      bitmap_copy (live_through, DF_LR_IN (bb));
+      bitmap_copy (live_through, df_get_live_in (bb));
       add_range_and_copies_from_move_list
 	(at_bb_start[bb->index], node, live_through, REG_FREQ_FROM_BB (bb));
-      bitmap_copy (live_through, DF_LR_OUT (bb));
+      bitmap_copy (live_through, df_get_live_out (bb));
       add_range_and_copies_from_move_list
 	(at_bb_end[bb->index], node, live_through, REG_FREQ_FROM_BB (bb));
       FOR_EACH_EDGE (e, ei, bb->succs)
 	{
-	  bitmap_and (live_through, DF_LR_IN (e->dest), DF_LR_OUT (bb));
+	  bitmap_and (live_through,
+		      df_get_live_in (e->dest), df_get_live_out (bb));
 	  add_range_and_copies_from_move_list
 	    ((move_t) e->aux, node, live_through,
 	     REG_FREQ_FROM_EDGE_FREQ (EDGE_FREQUENCY (e)));
Index: ira-color.c
===================================================================
--- ira-color.c	(revision 192426)
+++ ira-color.c	(working copy)
@@ -2014,8 +2014,8 @@ ira_loop_edge_freq (ira_loop_tree_node_t
       FOR_EACH_EDGE (e, ei, loop_node->loop->header->preds)
 	if (e->src != loop_node->loop->latch
 	    && (regno < 0
-		|| (bitmap_bit_p (DF_LR_OUT (e->src), regno)
-		    && bitmap_bit_p (DF_LR_IN (e->dest), regno))))
+		|| (bitmap_bit_p (df_get_live_out (e->src), regno)
+		    && bitmap_bit_p (df_get_live_in (e->dest), regno))))
 	  freq += EDGE_FREQUENCY (e);
     }
   else
@@ -2023,8 +2023,8 @@ ira_loop_edge_freq (ira_loop_tree_node_t
       edges = get_loop_exit_edges (loop_node->loop);
       FOR_EACH_VEC_ELT (edge, edges, i, e)
 	if (regno < 0
-	    || (bitmap_bit_p (DF_LR_OUT (e->src), regno)
-		&& bitmap_bit_p (DF_LR_IN (e->dest), regno)))
+	    || (bitmap_bit_p (df_get_live_out (e->src), regno)
+		&& bitmap_bit_p (df_get_live_in (e->dest), regno)))
 	  freq += EDGE_FREQUENCY (e);
       VEC_free (edge, heap, edges);
     }
Index: ira.c
===================================================================
--- ira.c	(revision 192426)
+++ ira.c	(working copy)
@@ -2337,16 +2337,23 @@ void
 mark_elimination (int from, int to)
 {
   basic_block bb;
+  bitmap r;
 
   FOR_EACH_BB (bb)
     {
-      /* We don't use LIVE info in IRA.  */
-      bitmap r = DF_LR_IN (bb);
-
-      if (REGNO_REG_SET_P (r, from))
+      r = DF_LR_IN (bb);
+      if (bitmap_bit_p (r, from))
+	{
+	  bitmap_clear_bit (r, from);
+	  bitmap_set_bit (r, to);
+	}
+      if (! df_live)
+        continue;
+      r = DF_LIVE_IN (bb);
+      if (bitmap_bit_p (r, from))
 	{
-	  CLEAR_REGNO_REG_SET (r, from);
-	  SET_REGNO_REG_SET (r, to);
+	  bitmap_clear_bit (r, from);
+	  bitmap_set_bit (r, to);
 	}
     }
 }
@@ -3194,10 +3201,12 @@ update_equiv_regs (void)
     {
       FOR_EACH_BB (bb)
 	{
-	  bitmap_and_compl_into (DF_LIVE_IN (bb), cleared_regs);
-	  bitmap_and_compl_into (DF_LIVE_OUT (bb), cleared_regs);
 	  bitmap_and_compl_into (DF_LR_IN (bb), cleared_regs);
 	  bitmap_and_compl_into (DF_LR_OUT (bb), cleared_regs);
+	  if (! df_live)
+	    continue;
+	  bitmap_and_compl_into (DF_LIVE_IN (bb), cleared_regs);
+	  bitmap_and_compl_into (DF_LIVE_OUT (bb), cleared_regs);
 	}
 
       /* Last pass - adjust debug insns referencing cleared regs.  */
@@ -3319,14 +3328,14 @@ build_insn_chain (void)
       CLEAR_REG_SET (live_relevant_regs);
       bitmap_clear (live_subregs_used);
 
-      EXECUTE_IF_SET_IN_BITMAP (DF_LR_OUT (bb), 0, i, bi)
+      EXECUTE_IF_SET_IN_BITMAP (df_get_live_out (bb), 0, i, bi)
 	{
 	  if (i >= FIRST_PSEUDO_REGISTER)
 	    break;
 	  bitmap_set_bit (live_relevant_regs, i);
 	}
 
-      EXECUTE_IF_SET_IN_BITMAP (DF_LR_OUT (bb),
+      EXECUTE_IF_SET_IN_BITMAP (df_get_live_out (bb),
 				FIRST_PSEUDO_REGISTER, i, bi)
 	{
 	  if (pseudo_for_reload_consideration_p (i))
@@ -4157,12 +4166,6 @@ ira (FILE *f)
   setup_prohibited_mode_move_regs ();
 
   df_note_add_problem ();
-
-  if (optimize == 1)
-    {
-      df_live_add_problem ();
-      df_live_set_all_dirty ();
-    }
 #ifdef ENABLE_CHECKING
   df->changeable_flags |= DF_VERIFY_SCHEDULED;
 #endif
@@ -4397,8 +4400,6 @@ do_reload (void)
      df_rescan_all_insns is not going to help here because it does not
      touch the artificial uses and defs.  */
   df_finish_pass (true);
-  if (optimize > 1)
-    df_live_add_problem ();
   df_scan_alloc (NULL);
   df_scan_blocks ();
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-14 11:42       ` Steven Bosscher
@ 2012-10-14 17:37         ` Vladimir Makarov
  2012-10-14 19:43           ` Steven Bosscher
  0 siblings, 1 reply; 14+ messages in thread
From: Vladimir Makarov @ 2012-10-14 17:37 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: GCC Patches, F. Kenneth Zadeck

On 12-10-14 6:16 AM, Steven Bosscher wrote:
> On Sat, Oct 13, 2012 at 11:12 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>> Ok for the idea.  If we have a problem later, we could fix it.  I'll look at
>> the next version of the patch when you send it to give your the final
>> approval.
> Great, thanks!
>
> Here is the updated patch, tested in the same way as the previous version.
>
>
Thanks, Steven.  IRA part is ok for me to commit.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-14 17:37         ` Vladimir Makarov
@ 2012-10-14 19:43           ` Steven Bosscher
  0 siblings, 0 replies; 14+ messages in thread
From: Steven Bosscher @ 2012-10-14 19:43 UTC (permalink / raw)
  To: Vladimir Makarov; +Cc: GCC Patches, F. Kenneth Zadeck

On Sun, Oct 14, 2012 at 7:19 PM, Vladimir Makarov wrote:
> Thanks, Steven.  IRA part is ok for me to commit.

Thanks, I've committed this as trunk r192440. I'm aware I'm on the
hook for fixing any fall-out :-)

Ciao!
Steven

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-13 21:41     ` Vladimir Makarov
  2012-10-14 11:42       ` Steven Bosscher
@ 2012-10-27 23:36       ` Steven Bosscher
  2012-10-28  0:38         ` Steven Bosscher
  1 sibling, 1 reply; 14+ messages in thread
From: Steven Bosscher @ 2012-10-27 23:36 UTC (permalink / raw)
  To: Vladimir Makarov; +Cc: GCC Patches, F. Kenneth Zadeck

On Sat, Oct 13, 2012 at 11:12 PM, Vladimir Makarov wrote:
> On 12-10-13 11:37 AM, Steven Bosscher wrote:
>>> LIVE is not used on purpose.  I added LIVE usage to the old RA about 10
>>> years ago (it was before DF-infrastructure).  Everything was ok until,
>>> somebody reported compiler crash on semantically wrong program (something
>>> with usage of uninitialized variable).  After that I removed this code.
>>
>> GCC was a completely different compiler 10 years ago. Handling of
>> uninitialized variables is radically different since tree-ssa, and
>> also the resource allocation side of the compiler has essentially been
>> rewritten (by you :-). Whatever broke in the compiler 10 years ago may
>> not be relevant anymore today.

It looks like the problems still exist, the PRs keep coming in
(PR55046, PR54961, PR54993, PR39607) with no obvious fixes.

So I'm going to revert this patch.

Ciao!
Steven

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-27 23:36       ` Steven Bosscher
@ 2012-10-28  0:38         ` Steven Bosscher
  2012-10-28 20:52           ` Vladimir Makarov
  0 siblings, 1 reply; 14+ messages in thread
From: Steven Bosscher @ 2012-10-28  0:38 UTC (permalink / raw)
  To: Vladimir Makarov; +Cc: GCC Patches, F. Kenneth Zadeck

On Sat, Oct 27, 2012 at 11:49 PM, Steven Bosscher wrote:
> So I'm going to revert this patch.

Or actually better, this lighter-weight patch. Will commit after the
usual testing.

        * ira.c (ira): Remove DF_LIVE if the problem is in the stack.
       (do_reload): Add it back at the end.

Index: ira.c
===================================================================
--- ira.c       (revision 192878)
+++ ira.c       (working copy)
@@ -4399,6 +4399,16 @@ ira (FILE *f)
   setup_prohibited_mode_move_regs ();

   df_note_add_problem ();
+
+  /* DF_LIVE can't be used in the register allocator, too many other
+     parts of the compiler depend on using the "classic" liveness
+     interpretation of the DF_LR problem.  See PR38711.
+     Remove the problem, so that we don't spend time updating it in
+     any of the df_analyze() calls during IRA/LRA.  */
+  if (optimize > 1)
+    df_remove_problem (df_live);
+  gcc_checking_assert (df_live == NULL);
+
 #ifdef ENABLE_CHECKING
   df->changeable_flags |= DF_VERIFY_SCHEDULED;
 #endif
@@ -4678,6 +4688,12 @@ do_reload (void)
   df_scan_alloc (NULL);
   df_scan_blocks ();

+  if (optimize > 1)
+    {
+      df_live_add_problem ();
+      df_live_set_all_dirty ();
+    }
+
   if (optimize)
     df_analyze ();

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher)
  2012-10-28  0:38         ` Steven Bosscher
@ 2012-10-28 20:52           ` Vladimir Makarov
  0 siblings, 0 replies; 14+ messages in thread
From: Vladimir Makarov @ 2012-10-28 20:52 UTC (permalink / raw)
  To: Steven Bosscher; +Cc: GCC Patches, F. Kenneth Zadeck

On 12-10-27 6:07 PM, Steven Bosscher wrote:
> On Sat, Oct 27, 2012 at 11:49 PM, Steven Bosscher wrote:
>> So I'm going to revert this patch.
> Or actually better, this lighter-weight patch. Will commit after the
> usual testing.
>
>
I returned to this problem several times and every time I failed to use 
LIVE info (may be I was not so persistent to solve the problem or had 
few time for this).  Your problem description helped me to remember what 
I saw many years ago.

May be this patch is even better because we don't need to spend time to 
calculate DF_LIVE although as Kenny wrote DF_LIVE might help for extreme 
tests as DF_LIVE is more compact than DF_LR.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-10-28 20:14 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-13 15:12 [PR38711] Use DF_LIVE in IRA if it available (for -O2 and higher) Steven Bosscher
2012-10-13 15:38 ` Vladimir Makarov
2012-10-13 15:51   ` Steven Bosscher
2012-10-13 21:41     ` Vladimir Makarov
2012-10-14 11:42       ` Steven Bosscher
2012-10-14 17:37         ` Vladimir Makarov
2012-10-14 19:43           ` Steven Bosscher
2012-10-27 23:36       ` Steven Bosscher
2012-10-28  0:38         ` Steven Bosscher
2012-10-28 20:52           ` Vladimir Makarov
2012-10-13 20:59 ` Eric Botcazou
2012-10-13 21:08   ` Steven Bosscher
2012-10-13 21:15     ` Eric Botcazou
2012-10-13 21:52       ` Steven Bosscher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).