patch to fix PR88282

Message ID e8caba76-8883-1108-4e10-78487b19ce8e@redhat.com
State New
Headers show
Series
  • patch to fix PR88282
Related show

Commit Message

Vladimir Makarov Dec. 4, 2018, 3:11 p.m.
The following patch fixes

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

   The patch was successfully bootstrapped and tested on 
x86/x86-64/ppc64/aarch64.

   Committed as rev. 266784.

Comments

Richard Sandiford Dec. 4, 2018, 4:13 p.m. | #1
Vladimir Makarov <vmakarov@redhat.com> writes:
>    The following patch fixes

>

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

>

>    The patch was successfully bootstrapped and tested on 

> x86/x86-64/ppc64/aarch64.

>

>    Committed as rev. 266784.

>

> Index: ChangeLog

> ===================================================================

> --- ChangeLog	(revision 266783)

> +++ ChangeLog	(working copy)

> @@ -1,3 +1,9 @@

> +2018-12-04  Vladimir Makarov  <vmakarov@redhat.com>

> +

> +	PR target/88282

> +	* ira-costs.c (exec): Try bigger class to use smaller register

> +	move cost.

> +

>  2018-12-04  Michael Ploujnikov  <michael.ploujnikov@oracle.com>

>  

>  	PR ipa/88297

> Index: ira-costs.c

> ===================================================================

> --- ira-costs.c	(revision 266678)

> +++ ira-costs.c	(working copy)

> @@ -1314,28 +1314,50 @@ record_operand_costs (rtx_insn *insn, en

>  	  machine_mode mode = GET_MODE (SET_SRC (set));

>  	  cost_classes_t cost_classes_ptr = regno_cost_classes[regno];

>  	  enum reg_class *cost_classes = cost_classes_ptr->classes;

> -	  reg_class_t rclass, hard_reg_class, pref_class;

> +	  reg_class_t rclass, hard_reg_class, pref_class, bigger_hard_reg_class;

>  	  int cost, k;

> +	  move_table *move_costs;

>  	  bool dead_p = find_regno_note (insn, REG_DEAD, REGNO (src));

>  

>  	  ira_init_register_move_cost_if_necessary (mode);

> +	  move_costs = ira_register_move_cost[mode];

>  	  hard_reg_class = REGNO_REG_CLASS (other_regno);

> +	  bigger_hard_reg_class = ira_pressure_class_translate[hard_reg_class];

> +	  if (bigger_hard_reg_class == NO_REGS

> +	      && (other_regno == STACK_POINTER_REGNUM

> +#ifdef STATIC_CHAIN_REGNUM

> +		  || other_regno == STATIC_CHAIN_REGNUM

> +#endif

> +		  || other_regno == FRAME_POINTER_REGNUM

> +		  || other_regno == HARD_FRAME_POINTER_REGNUM))

> +	    bigger_hard_reg_class = GENERAL_REGS;

>  	  /* Target code may return any cost for mode which does not

>  	     fit the the hard reg class (e.g. DImode for AREG on

>  	     i386).  Check this and use a bigger class to get the

>  	     right cost.  */

>  	  if (! ira_hard_reg_in_set_p (other_regno, mode,

>  				       reg_class_contents[hard_reg_class]))

> -	    hard_reg_class = ira_pressure_class_translate[hard_reg_class];

> +	    hard_reg_class = bigger_hard_reg_class;

>  	  i = regno == (int) REGNO (src) ? 1 : 0;

>  	  for (k = cost_classes_ptr->num - 1; k >= 0; k--)

>  	    {

>  	      rclass = cost_classes[k];

> -	      cost = ((i == 0

> -		       ? ira_register_move_cost[mode][hard_reg_class][rclass]

> -		       : ira_register_move_cost[mode][rclass][hard_reg_class])

> -		      * frequency);

> -	      op_costs[i]->cost[k] = cost;

> +	      cost = (i == 0

> +		      ? move_costs[hard_reg_class][rclass]

> +		      : move_costs[rclass][hard_reg_class]);

> +	      /* Target code might define wrong big costs for smaller

> +		 reg classes or reg classes containing only fixed hard

> +		 regs.  Try a bigger class.  */

> +	      if (bigger_hard_reg_class != hard_reg_class)

> +		{

> +		  int cost2 = (i == 0

> +			       ? move_costs[bigger_hard_reg_class][rclass]

> +			       : move_costs[rclass][bigger_hard_reg_class]);

> +		  if (cost2 < cost)

> +		    cost = cost2;

> +		}

> +	      

> +	      op_costs[i]->cost[k] = cost * frequency;

>  	      /* If we have assigned a class to this allocno in our

>  		 first pass, add a cost to this alternative

>  		 corresponding to what we would add if this allocno


This seems like a hack to me.  I don't see any reason why
FRAME_POINTER_REGNUM etc. have to be in GENERAL_REGS, so picking
that seems arbitrary.  And is there a reason not to include
ARG_POINTER_REGNUM or PIC_OFFSET_TABLE_REGNUM?

It sounds from the comments in the PR that the backend isn't defining
its costs correctly.  If so, what specifically does it get wrong?
I think it would be better to fix the backends than accumulate more
workarounds like this.

Thanks,
Richard

Patch

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 266783)
+++ ChangeLog	(working copy)
@@ -1,3 +1,9 @@ 
+2018-12-04  Vladimir Makarov  <vmakarov@redhat.com>
+
+	PR target/88282
+	* ira-costs.c (exec): Try bigger class to use smaller register
+	move cost.
+
 2018-12-04  Michael Ploujnikov  <michael.ploujnikov@oracle.com>
 
 	PR ipa/88297
Index: ira-costs.c
===================================================================
--- ira-costs.c	(revision 266678)
+++ ira-costs.c	(working copy)
@@ -1314,28 +1314,50 @@  record_operand_costs (rtx_insn *insn, en
 	  machine_mode mode = GET_MODE (SET_SRC (set));
 	  cost_classes_t cost_classes_ptr = regno_cost_classes[regno];
 	  enum reg_class *cost_classes = cost_classes_ptr->classes;
-	  reg_class_t rclass, hard_reg_class, pref_class;
+	  reg_class_t rclass, hard_reg_class, pref_class, bigger_hard_reg_class;
 	  int cost, k;
+	  move_table *move_costs;
 	  bool dead_p = find_regno_note (insn, REG_DEAD, REGNO (src));
 
 	  ira_init_register_move_cost_if_necessary (mode);
+	  move_costs = ira_register_move_cost[mode];
 	  hard_reg_class = REGNO_REG_CLASS (other_regno);
+	  bigger_hard_reg_class = ira_pressure_class_translate[hard_reg_class];
+	  if (bigger_hard_reg_class == NO_REGS
+	      && (other_regno == STACK_POINTER_REGNUM
+#ifdef STATIC_CHAIN_REGNUM
+		  || other_regno == STATIC_CHAIN_REGNUM
+#endif
+		  || other_regno == FRAME_POINTER_REGNUM
+		  || other_regno == HARD_FRAME_POINTER_REGNUM))
+	    bigger_hard_reg_class = GENERAL_REGS;
 	  /* Target code may return any cost for mode which does not
 	     fit the the hard reg class (e.g. DImode for AREG on
 	     i386).  Check this and use a bigger class to get the
 	     right cost.  */
 	  if (! ira_hard_reg_in_set_p (other_regno, mode,
 				       reg_class_contents[hard_reg_class]))
-	    hard_reg_class = ira_pressure_class_translate[hard_reg_class];
+	    hard_reg_class = bigger_hard_reg_class;
 	  i = regno == (int) REGNO (src) ? 1 : 0;
 	  for (k = cost_classes_ptr->num - 1; k >= 0; k--)
 	    {
 	      rclass = cost_classes[k];
-	      cost = ((i == 0
-		       ? ira_register_move_cost[mode][hard_reg_class][rclass]
-		       : ira_register_move_cost[mode][rclass][hard_reg_class])
-		      * frequency);
-	      op_costs[i]->cost[k] = cost;
+	      cost = (i == 0
+		      ? move_costs[hard_reg_class][rclass]
+		      : move_costs[rclass][hard_reg_class]);
+	      /* Target code might define wrong big costs for smaller
+		 reg classes or reg classes containing only fixed hard
+		 regs.  Try a bigger class.  */
+	      if (bigger_hard_reg_class != hard_reg_class)
+		{
+		  int cost2 = (i == 0
+			       ? move_costs[bigger_hard_reg_class][rclass]
+			       : move_costs[rclass][bigger_hard_reg_class]);
+		  if (cost2 < cost)
+		    cost = cost2;
+		}
+	      
+	      op_costs[i]->cost[k] = cost * frequency;
 	      /* If we have assigned a class to this allocno in our
 		 first pass, add a cost to this alternative
 		 corresponding to what we would add if this allocno
@@ -1350,7 +1372,7 @@  record_operand_costs (rtx_insn *insn, en
 		  else if (ira_reg_class_intersect[pref_class][rclass]
 			   == NO_REGS)
 		    op_costs[i]->cost[k]
-		      += (ira_register_move_cost[mode][pref_class][rclass]
+		      += (move_costs[pref_class][rclass]
 			  * frequency);
 		}
 	      /* If this insn is a single set copying operand 1 to