Patchwork Make mcf.c and ira-conflicts.c not include tree.h

login
register
mail settings
Submitter Steven Bosscher
Date July 8, 2012, 12:44 p.m.
Message ID <CABu31nOhJ_FYYjf0nPvw4Q4ZNH=nBMYBc13Mk4GikwxG2tEdGg@mail.gmail.com>
Download mbox | patch
Permalink /patch/169644/
State New
Headers show

Comments

Steven Bosscher - July 8, 2012, 12:44 p.m.
Hello,

IMHO, no RTL pass implementation file should have to include tree.h.
So ira-conflicts.c shouldn't need tree.h. The reason it needed it, was
because there is code to avoid putting user variables in callee
clobbered registers, and ira-conflict.c looked at DECL_ARTIFICIAL
(REG_EXPR (reg)). But it can just look at REG_USERVAR_P(reg) instead,
which is simper and cleaner.

Likewise, IMHO no CFG implementation files should need tree.h, because
the CFG should be IR agnostic. But mcf.c needed tree.h for
lang_hooks.decl_printable_name from langhooks.h. It can use
current_function_name() instead.

Bootstrapped and tested on powerpc64-unknown-linux-gnu. OK for trunk?

Ciao!
Steven

	* mcf.c: Do not include tm.h, tree.h, and langhooks.h.
	(dump_fixup_graph): Use current_function_name.
	(adjust_cfg_counts): Likewise.
	* ira-conflicts.c: Do not include tree.h.
	(ira_build_conflicts): Use REG_USERVAR_P instead of DECL_ARTIFICIAL.
Richard Guenther - July 8, 2012, 6:03 p.m.
Steven Bosscher <stevenb.gcc@gmail.com> wrote:

>Hello,
>
>IMHO, no RTL pass implementation file should have to include tree.h.
>So ira-conflicts.c shouldn't need tree.h. The reason it needed it, was
>because there is code to avoid putting user variables in callee
>clobbered registers, and ira-conflict.c looked at DECL_ARTIFICIAL
>(REG_EXPR (reg)). But it can just look at REG_USERVAR_P(reg) instead,
>which is simper and cleaner.
>
>Likewise, IMHO no CFG implementation files should need tree.h, because
>the CFG should be IR agnostic. But mcf.c needed tree.h for
>lang_hooks.decl_printable_name from langhooks.h. It can use
>current_function_name() instead.
>
>Bootstrapped and tested on powerpc64-unknown-linux-gnu. OK for trunk?

Ok.

Thanks,
Richard.

>Ciao!
>Steven
>
>	* mcf.c: Do not include tm.h, tree.h, and langhooks.h.
>	(dump_fixup_graph): Use current_function_name.
>	(adjust_cfg_counts): Likewise.
>	* ira-conflicts.c: Do not include tree.h.
>	(ira_build_conflicts): Use REG_USERVAR_P instead of DECL_ARTIFICIAL.
>
>Index: mcf.c
>===================================================================
>--- mcf.c	(revision 189359)
>+++ mcf.c	(working copy)
>@@ -46,13 +46,8 @@ along with GCC; see the file COPYING3.  If not see
> #include "config.h"
> #include "system.h"
> #include "coretypes.h"
>-#include "tm.h"
> #include "basic-block.h"
>-#include "tree.h"		/* FIXME: Only for langhooks.h.  */
>-#include "langhooks.h"
>-#include "tree.h"
> #include "gcov-io.h"
>-
> #include "profile.h"
>
> /* CAP_INFINITY: Constant to represent infinite capacity.  */
>@@ -290,7 +285,7 @@ dump_fixup_graph (FILE *file, fixup_graph_type *fi
>   fnum_edges = fixup_graph->num_edges;
>
>   fprintf (file, "\nDump fixup graph for %s(): %s.\n",
>-	   lang_hooks.decl_printable_name (current_function_decl, 2), msg);
>+	   current_function_name (), msg);
>   fprintf (file,
> 	   "There are %d vertices and %d edges. new_exit_index is %d.\n\n",
> 	   fnum_vertices, fnum_edges, fixup_graph->new_exit_index);
>@@ -1280,8 +1275,8 @@ adjust_cfg_counts (fixup_graph_type *fixup_graph)
>   if (dump_file)
>     {
>       fprintf (dump_file, "\nCheck %s() CFG flow conservation:\n",
>-           lang_hooks.decl_printable_name (current_function_decl, 2));
>-      FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR->next_bb, EXIT_BLOCK_PTR,
>next_bb)
>+	       current_function_name ());
>+      FOR_EACH_BB (bb)
>         {
>           if ((bb->count != sum_edge_counts (bb->preds))
>                || (bb->count != sum_edge_counts (bb->succs)))
>Index: ira-conflicts.c
>===================================================================
>--- ira-conflicts.c	(revision 189359)
>+++ ira-conflicts.c	(working copy)
>@@ -25,7 +25,6 @@ along with GCC; see the file COPYING3.  If not see
> #include "tm.h"
> #include "regs.h"
> #include "rtl.h"
>-#include "tree.h"		/* For DECL_ARTIFICIAL and friends.  */
> #include "tm_p.h"
> #include "target.h"
> #include "flags.h"
>@@ -893,17 +892,12 @@ ira_build_conflicts (void)
>       for (i = 0; i < n; i++)
> 	{
> 	  ira_object_t obj = ALLOCNO_OBJECT (a, i);
>-	  reg_attrs *attrs = REG_ATTRS (regno_reg_rtx [ALLOCNO_REGNO (a)]);
>-	  tree decl;
>+	  rtx allocno_reg = regno_reg_rtx [ALLOCNO_REGNO (a)];
>
> 	  if ((! flag_caller_saves && ALLOCNO_CALLS_CROSSED_NUM (a) != 0)
> 	      /* For debugging purposes don't put user defined variables in
> 		 callee-clobbered registers.  */
>-	      || (optimize == 0
>-		  && attrs != NULL
>-		  && (decl = attrs->decl) != NULL
>-		  && VAR_OR_FUNCTION_DECL_P (decl)
>-		  && ! DECL_ARTIFICIAL (decl)))
>+	      || (optimize == 0 && REG_USERVAR_P (allocno_reg)))
> 	    {
> 	      IOR_HARD_REG_SET (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj),
> 				call_used_reg_set);
Jakub Jelinek - July 16, 2012, 7:35 p.m.
On Sun, Jul 08, 2012 at 02:44:15PM +0200, Steven Bosscher wrote:
> IMHO, no RTL pass implementation file should have to include tree.h.
> So ira-conflicts.c shouldn't need tree.h. The reason it needed it, was
> because there is code to avoid putting user variables in callee
> clobbered registers, and ira-conflict.c looked at DECL_ARTIFICIAL
> (REG_EXPR (reg)). But it can just look at REG_USERVAR_P(reg) instead,
> which is simper and cleaner.

But it regresses PR53948.

> --- ira-conflicts.c	(revision 189359)
> +++ ira-conflicts.c	(working copy)
> @@ -25,7 +25,6 @@ along with GCC; see the file COPYING3.  If not see
>  #include "tm.h"
>  #include "regs.h"
>  #include "rtl.h"
> -#include "tree.h"		/* For DECL_ARTIFICIAL and friends.  */
>  #include "tm_p.h"
>  #include "target.h"
>  #include "flags.h"
> @@ -893,17 +892,12 @@ ira_build_conflicts (void)
>        for (i = 0; i < n; i++)
>  	{
>  	  ira_object_t obj = ALLOCNO_OBJECT (a, i);
> -	  reg_attrs *attrs = REG_ATTRS (regno_reg_rtx [ALLOCNO_REGNO (a)]);
> -	  tree decl;
> +	  rtx allocno_reg = regno_reg_rtx [ALLOCNO_REGNO (a)];
> 
>  	  if ((! flag_caller_saves && ALLOCNO_CALLS_CROSSED_NUM (a) != 0)
>  	      /* For debugging purposes don't put user defined variables in
>  		 callee-clobbered registers.  */
> -	      || (optimize == 0
> -		  && attrs != NULL
> -		  && (decl = attrs->decl) != NULL
> -		  && VAR_OR_FUNCTION_DECL_P (decl)
> -		  && ! DECL_ARTIFICIAL (decl)))
> +	      || (optimize == 0 && REG_USERVAR_P (allocno_reg)))
>  	    {
>  	      IOR_HARD_REG_SET (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj),
>  				call_used_reg_set);

	Jakub
Steven Bosscher - July 16, 2012, 7:47 p.m.
On Mon, Jul 16, 2012 at 9:35 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Sun, Jul 08, 2012 at 02:44:15PM +0200, Steven Bosscher wrote:
>> IMHO, no RTL pass implementation file should have to include tree.h.
>> So ira-conflicts.c shouldn't need tree.h. The reason it needed it, was
>> because there is code to avoid putting user variables in callee
>> clobbered registers, and ira-conflict.c looked at DECL_ARTIFICIAL
>> (REG_EXPR (reg)). But it can just look at REG_USERVAR_P(reg) instead,
>> which is simper and cleaner.
>
> But it regresses PR53948.

And your proposed solution in the PR is to revert the patch? I don't
see how that helps, and frankly I don't think it's a constructive
attitude.

The variable that is optimized out is a user variable. Instead of
reverting my patch, we should find out why REG_USERVAR_P isn't set for
it.

Ciao!
Steven

Patch

Index: mcf.c
===================================================================
--- mcf.c	(revision 189359)
+++ mcf.c	(working copy)
@@ -46,13 +46,8 @@  along with GCC; see the file COPYING3.  If not see
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
-#include "tm.h"
 #include "basic-block.h"
-#include "tree.h"		/* FIXME: Only for langhooks.h.  */
-#include "langhooks.h"
-#include "tree.h"
 #include "gcov-io.h"
-
 #include "profile.h"

 /* CAP_INFINITY: Constant to represent infinite capacity.  */
@@ -290,7 +285,7 @@  dump_fixup_graph (FILE *file, fixup_graph_type *fi
   fnum_edges = fixup_graph->num_edges;

   fprintf (file, "\nDump fixup graph for %s(): %s.\n",
-	   lang_hooks.decl_printable_name (current_function_decl, 2), msg);
+	   current_function_name (), msg);
   fprintf (file,
 	   "There are %d vertices and %d edges. new_exit_index is %d.\n\n",
 	   fnum_vertices, fnum_edges, fixup_graph->new_exit_index);
@@ -1280,8 +1275,8 @@  adjust_cfg_counts (fixup_graph_type *fixup_graph)
   if (dump_file)
     {
       fprintf (dump_file, "\nCheck %s() CFG flow conservation:\n",
-           lang_hooks.decl_printable_name (current_function_decl, 2));
-      FOR_BB_BETWEEN (bb, ENTRY_BLOCK_PTR->next_bb, EXIT_BLOCK_PTR, next_bb)
+	       current_function_name ());
+      FOR_EACH_BB (bb)
         {
           if ((bb->count != sum_edge_counts (bb->preds))
                || (bb->count != sum_edge_counts (bb->succs)))
Index: ira-conflicts.c
===================================================================
--- ira-conflicts.c	(revision 189359)
+++ ira-conflicts.c	(working copy)
@@ -25,7 +25,6 @@  along with GCC; see the file COPYING3.  If not see
 #include "tm.h"
 #include "regs.h"
 #include "rtl.h"
-#include "tree.h"		/* For DECL_ARTIFICIAL and friends.  */
 #include "tm_p.h"
 #include "target.h"
 #include "flags.h"
@@ -893,17 +892,12 @@  ira_build_conflicts (void)
       for (i = 0; i < n; i++)
 	{
 	  ira_object_t obj = ALLOCNO_OBJECT (a, i);
-	  reg_attrs *attrs = REG_ATTRS (regno_reg_rtx [ALLOCNO_REGNO (a)]);
-	  tree decl;
+	  rtx allocno_reg = regno_reg_rtx [ALLOCNO_REGNO (a)];

 	  if ((! flag_caller_saves && ALLOCNO_CALLS_CROSSED_NUM (a) != 0)
 	      /* For debugging purposes don't put user defined variables in
 		 callee-clobbered registers.  */
-	      || (optimize == 0
-		  && attrs != NULL
-		  && (decl = attrs->decl) != NULL
-		  && VAR_OR_FUNCTION_DECL_P (decl)
-		  && ! DECL_ARTIFICIAL (decl)))
+	      || (optimize == 0 && REG_USERVAR_P (allocno_reg)))
 	    {
 	      IOR_HARD_REG_SET (OBJECT_TOTAL_CONFLICT_HARD_REGS (obj),
 				call_used_reg_set);