patch to fix PR59511

Vladimir Makarov vmakarov@redhat.com
Wed Jan 15 17:34:00 GMT 2014


The following patch fixes:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59511

Register move cost calculations were too conservative for some cases
and it resulted into overflows.  The patch fixes also some wrong
frequencies usage I found.

The patch has very small affect on SPEC2000.  Only 7 programs out of
26 has a different code for -mtune=corei7.  For -mtune=generic, it is
only 3 programs. And there is no visible changes in performance for
given programs.  So I'd say this PR is not worth P1 status.

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

Committed as rev. 206636.


2014-01-15  Vladimir Makarov  <vmakarov@redhat.com>

         PR rtl-optimization/59511
         * ira.c (ira_init_register_move_cost): Use memory costs for some
         cases of register move cost calculations.
         * lra-constraints.c (lra_constraints): Use REG_FREQ_FROM_BB
         instead of BB frequency.
         * lra-coalesce.c (move_freq_compare_func, lra_coalesce): Ditto.
         * lra-assigns.c (find_hard_regno_for): Ditto.
-------------- next part --------------
Index: ira.c
===================================================================
--- ira.c	(revision 206609)
+++ ira.c	(working copy)
@@ -1574,21 +1574,30 @@ ira_init_register_move_cost (enum machin
 	      && ira_may_move_out_cost[mode] == NULL);
   ira_assert (have_regs_of_mode[mode]);
   for (cl1 = 0; cl1 < N_REG_CLASSES; cl1++)
-    if (contains_reg_of_mode[cl1][mode])
-      for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
-	{
-	  int cost;
-	  if (!contains_reg_of_mode[cl2][mode])
-	    cost = 65535;
-	  else
-	    {
-	      cost = register_move_cost (mode, (enum reg_class) cl1,
-					 (enum reg_class) cl2);
-	      ira_assert (cost < 65535);
-	    }
-	  all_match &= (last_move_cost[cl1][cl2] == cost);
-	  last_move_cost[cl1][cl2] = cost;
-	}
+    for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
+      {
+	int cost;
+	if (!contains_reg_of_mode[cl1][mode]
+	    || !contains_reg_of_mode[cl2][mode])
+	  {
+	    if ((ira_reg_class_max_nregs[cl1][mode]
+		 > ira_class_hard_regs_num[cl1])
+		|| (ira_reg_class_max_nregs[cl2][mode]
+		    > ira_class_hard_regs_num[cl2]))
+	      cost = 65535;
+	    else
+	      cost = (ira_memory_move_cost[mode][cl1][0]
+		      + ira_memory_move_cost[mode][cl2][1]);
+	  }
+	else
+	  {
+	    cost = register_move_cost (mode, (enum reg_class) cl1,
+				       (enum reg_class) cl2);
+	    ira_assert (cost < 65535);
+	  }
+	all_match &= (last_move_cost[cl1][cl2] == cost);
+	last_move_cost[cl1][cl2] = cost;
+      }
   if (all_match && last_mode_for_init_move_cost != -1)
     {
       ira_register_move_cost[mode]
@@ -1604,58 +1613,51 @@ ira_init_register_move_cost (enum machin
   ira_may_move_in_cost[mode] = XNEWVEC (move_table, N_REG_CLASSES);
   ira_may_move_out_cost[mode] = XNEWVEC (move_table, N_REG_CLASSES);
   for (cl1 = 0; cl1 < N_REG_CLASSES; cl1++)
-    if (contains_reg_of_mode[cl1][mode])
-      for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
-	{
-	  int cost;
-	  enum reg_class *p1, *p2;
-
-	  if (last_move_cost[cl1][cl2] == 65535)
-	    {
-	      ira_register_move_cost[mode][cl1][cl2] = 65535;
-	      ira_may_move_in_cost[mode][cl1][cl2] = 65535;
-	      ira_may_move_out_cost[mode][cl1][cl2] = 65535;
-	    }
-	  else
-	    {
-	      cost = last_move_cost[cl1][cl2];
-
-	      for (p2 = &reg_class_subclasses[cl2][0];
-		   *p2 != LIM_REG_CLASSES; p2++)
-		if (ira_class_hard_regs_num[*p2] > 0
-		    && (ira_reg_class_max_nregs[*p2][mode]
-			<= ira_class_hard_regs_num[*p2]))
-		  cost = MAX (cost, ira_register_move_cost[mode][cl1][*p2]);
-
-	      for (p1 = &reg_class_subclasses[cl1][0];
-		   *p1 != LIM_REG_CLASSES; p1++)
-		if (ira_class_hard_regs_num[*p1] > 0
-		    && (ira_reg_class_max_nregs[*p1][mode]
-			<= ira_class_hard_regs_num[*p1]))
-		  cost = MAX (cost, ira_register_move_cost[mode][*p1][cl2]);
-
-	      ira_assert (cost <= 65535);
-	      ira_register_move_cost[mode][cl1][cl2] = cost;
-
-	      if (ira_class_subset_p[cl1][cl2])
-		ira_may_move_in_cost[mode][cl1][cl2] = 0;
-	      else
-		ira_may_move_in_cost[mode][cl1][cl2] = cost;
-
-	      if (ira_class_subset_p[cl2][cl1])
-		ira_may_move_out_cost[mode][cl1][cl2] = 0;
-	      else
-		ira_may_move_out_cost[mode][cl1][cl2] = cost;
-	    }
-	}
-    else
-      for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
-	{
-	  ira_register_move_cost[mode][cl1][cl2] = 65535;
-	  ira_may_move_in_cost[mode][cl1][cl2] = 65535;
-	  ira_may_move_out_cost[mode][cl1][cl2] = 65535;
-	}
+    for (cl2 = 0; cl2 < N_REG_CLASSES; cl2++)
+      {
+	int cost;
+	enum reg_class *p1, *p2;
+	
+	if (last_move_cost[cl1][cl2] == 65535)
+	  {
+	    ira_register_move_cost[mode][cl1][cl2] = 65535;
+	    ira_may_move_in_cost[mode][cl1][cl2] = 65535;
+	    ira_may_move_out_cost[mode][cl1][cl2] = 65535;
+	  }
+	else
+	  {
+	    cost = last_move_cost[cl1][cl2];
+	    
+	    for (p2 = &reg_class_subclasses[cl2][0];
+		 *p2 != LIM_REG_CLASSES; p2++)
+	      if (ira_class_hard_regs_num[*p2] > 0
+		  && (ira_reg_class_max_nregs[*p2][mode]
+		      <= ira_class_hard_regs_num[*p2]))
+		cost = MAX (cost, ira_register_move_cost[mode][cl1][*p2]);
+	    
+	    for (p1 = &reg_class_subclasses[cl1][0];
+		 *p1 != LIM_REG_CLASSES; p1++)
+	      if (ira_class_hard_regs_num[*p1] > 0
+		  && (ira_reg_class_max_nregs[*p1][mode]
+		      <= ira_class_hard_regs_num[*p1]))
+		cost = MAX (cost, ira_register_move_cost[mode][*p1][cl2]);
+	    
+	    ira_assert (cost <= 65535);
+	    ira_register_move_cost[mode][cl1][cl2] = cost;
+	    
+	    if (ira_class_subset_p[cl1][cl2])
+	      ira_may_move_in_cost[mode][cl1][cl2] = 0;
+	    else
+	      ira_may_move_in_cost[mode][cl1][cl2] = cost;
+	    
+	    if (ira_class_subset_p[cl2][cl1])
+	      ira_may_move_out_cost[mode][cl1][cl2] = 0;
+	    else
+	      ira_may_move_out_cost[mode][cl1][cl2] = cost;
+	  }
+      }
 }
+
 

 
 /* This is called once during compiler work.  It sets up
Index: lra-assigns.c
===================================================================
--- lra-assigns.c	(revision 206609)
+++ lra-assigns.c	(working copy)
@@ -612,7 +612,9 @@ find_hard_regno_for (int regno, int *cos
 		&& ! df_regs_ever_live_p (hard_regno + j))
 	      /* It needs save restore.	 */
 	      hard_regno_costs[hard_regno]
-		+= 2 * ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb->frequency + 1;
+		+= (2
+		    * REG_FREQ_FROM_BB (ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb)
+		    + 1);
 	  priority = targetm.register_priority (hard_regno);
 	  if (best_hard_regno < 0 || hard_regno_costs[hard_regno] < best_cost
 	      || (hard_regno_costs[hard_regno] == best_cost
Index: lra-coalesce.c
===================================================================
--- lra-coalesce.c	(revision 206609)
+++ lra-coalesce.c	(working copy)
@@ -79,8 +79,8 @@ move_freq_compare_func (const void *v1p,
   rtx mv2 = *(const rtx *) v2p;
   int pri1, pri2;
 
-  pri1 = BLOCK_FOR_INSN (mv1)->frequency;
-  pri2 = BLOCK_FOR_INSN (mv2)->frequency;
+  pri1 = REG_FREQ_FROM_BB (BLOCK_FOR_INSN (mv1));
+  pri2 = REG_FREQ_FROM_BB (BLOCK_FOR_INSN (mv2));
   if (pri2 - pri1)
     return pri2 - pri1;
 
@@ -277,7 +277,7 @@ lra_coalesce (void)
 	    fprintf
 	      (lra_dump_file, "      Coalescing move %i:r%d-r%d (freq=%d)\n",
 	       INSN_UID (mv), sregno, dregno,
-	       BLOCK_FOR_INSN (mv)->frequency);
+	       REG_FREQ_FROM_BB (BLOCK_FOR_INSN (mv)));
 	  /* We updated involved_insns_bitmap when doing the merge.  */
 	}
       else if (!(lra_intersected_live_ranges_p
@@ -291,7 +291,7 @@ lra_coalesce (void)
 	       "  Coalescing move %i:r%d(%d)-r%d(%d) (freq=%d)\n",
 	       INSN_UID (mv), sregno, ORIGINAL_REGNO (SET_SRC (set)),
 	       dregno, ORIGINAL_REGNO (SET_DEST (set)),
-	       BLOCK_FOR_INSN (mv)->frequency);
+	       REG_FREQ_FROM_BB (BLOCK_FOR_INSN (mv)));
 	  bitmap_ior_into (&involved_insns_bitmap,
 			   &lra_reg_info[sregno].insn_bitmap);
 	  bitmap_ior_into (&involved_insns_bitmap,
@@ -316,7 +316,8 @@ lra_coalesce (void)
 		/* Coalesced move.  */
 		if (lra_dump_file != NULL)
 		  fprintf (lra_dump_file, "	 Removing move %i (freq=%d)\n",
-			 INSN_UID (insn), BLOCK_FOR_INSN (insn)->frequency);
+			   INSN_UID (insn),
+			   REG_FREQ_FROM_BB (BLOCK_FOR_INSN (insn)));
 		lra_set_insn_deleted (insn);
 	      }
 	  }
Index: lra-constraints.c
===================================================================
--- lra-constraints.c	(revision 206609)
+++ lra-constraints.c	(working copy)
@@ -4077,7 +4077,7 @@ lra_constraints (bool first_p)
 		      fprintf (lra_dump_file,
 			       "      Removing equiv init insn %i (freq=%d)\n",
 			       INSN_UID (curr_insn),
-			       BLOCK_FOR_INSN (curr_insn)->frequency);
+			       REG_FREQ_FROM_BB (BLOCK_FOR_INSN (curr_insn)));
 		      dump_insn_slim (lra_dump_file, curr_insn);
 		    }
 		  if (contains_reg_p (x, true, false))


More information about the Gcc-patches mailing list