[PATCH] Improve TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS callback

[Wilco Dijkstra <Wilco dot Dijkstra at arm dot com>]

Improve TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS target hook. It turns out there
is another case where the register allocator uses the union of register classes
without checking that the cost of the resulting register class is lower than
both (see https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01765.html ).  

This happens when the cost of the best and alternative class are both lower than the
memory cost.  In this case we typically end up with ALL_REGS as the allocno
class, which almost invariably results in bad allocations with many redundant
int<->FP moves (which are expensive on various cores).  AArch64 is affected by
this significantly due to supporting many scalar integer operations in SIMD.

Currently the TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook on AArch64 forces the
class to GENERAL_REGS if the allocno class is ALL_REGS and the register has an
integer mode.  This is bad if the best class happens to be FP_REGS.  To handle
this case as well, an extra argument is needed in the hook to pass the best
class.  If the allocno class is ALL_REGS, but the best class isn't, we use the
best class instead (rather than using the mode).

Previously this might happen (for the vdup_lane.c regression discussed in the linked post):

r79: preferred FP_REGS, alternative GENERAL_REGS, allocno GENERAL_REGS
     a1 (r79,l0) best GENERAL_REGS, allocno GENERAL_REGS

a1(r79,l0) costs: CALLER_SAVE_REGS:5000,5000 GENERAL_REGS:5000,5000 FP_LO_REGS:0,0 FP_REGS:0,0 ALL_REGS:10000,10000 MEM:9000,9000

The proposed allocno is ALL_REGS (despite having the highest cost!) and is then
forced by the hook to GENERAL_REGS because r79 has integer mode.  However FP_REGS
has the lowest cost.  After this patch the choice is as follows:

r79: preferred FP_REGS, alternative GENERAL_REGS, allocno FP_REGS
     a1 (r79,l0) best FP_REGS, allocno FP_REGS

As a result it is now no longer a requirement to use register move costs that 
are larger than the memory move cost.  So it will be feasible to use realistic
costs for both without a huge penalty.


ChangeLog:
2016-01-22  Wilco Dijkstra  <wdijkstr@arm.com>

gcc/
	* ira-costs.c (find_costs_and_classes): Add extra argument.
	* target.def (ira_change_pseudo_allocno_class): Add parameter.
	* targhooks.h (ira_change_pseudo_allocno_class): Likewise.
	* targhooks.c (ira_change_pseudo_allocno_class): Likewise.
	* config/aarch64/aarch64.c (aarch64_ira_change_pseudo_allocno_class)
	Add best_class parameter, and return it if not ALL_REGS.
	* config/mips/mips.c (mips_ira_change_pseudo_allocno_class): Add parameter.
	* doc/tm.texi (TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS): Update target hook.

---

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 2584f16d345b3d015d577dd28c08a73ee3e0b0fb..f3f750ed9486a583df3753e4e67584432e9586bb 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -726,18 +726,31 @@ aarch64_err_no_fpadvsimd (machine_mode mode, const char *msg)
 
 /* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS.
    The register allocator chooses ALL_REGS if FP_REGS and GENERAL_REGS have
-   the same cost even if ALL_REGS has a much larger cost.  This results in bad
-   allocations and spilling.  To avoid this we force the class to GENERAL_REGS
-   if the mode is integer.  */
+   the same cost even if ALL_REGS has a much larger cost.  ALL_REGS is also
+   used if the cost of both FP_REGS and GENERAL_REGS is lower than the memory
+   cost (in this case the best class is the lowest cost one).  Using ALL_REGS
+   irrespectively of its cost results in bad allocations with many redundant
+   int<->FP moves which are expensive on various cores.
+   To avoid this we don't allow ALL_REGS as the allocno class, but force a
+   decision between FP_REGS and GENERAL_REGS.  We use the allocno class if it
+   isn't ALL_REGS.  Similarly, use the best class if it isn't ALL_REGS.
+   Otherwise set the allocno class depending on the mode.
+   The result of this is that it is no longer inefficient to have a higher
+   memory move cost than the register move cost.
+*/
 
 static reg_class_t
-aarch64_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
+aarch64_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class,
+					 reg_class_t best_class)
 {
   enum machine_mode mode;
 
   if (allocno_class != ALL_REGS)
     return allocno_class;
 
+  if (best_class != ALL_REGS)
+    return best_class;
+
   mode = PSEUDO_REGNO_MODE (regno);
   return FLOAT_MODE_P (mode) || VECTOR_MODE_P (mode) ? FP_REGS : GENERAL_REGS;
 }
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 6145944fbf7584fbafc6915cbe5fd855e1d21dc8..95f8bc06e03b605757d191a4448d974a587d623c 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19883,7 +19883,8 @@ mips_lra_p (void)
 /* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS.  */
 
 static reg_class_t
-mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
+mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class,
+				      reg_class_t best_class ATTRIBUTE_UNUSED)
 {
   /* LRA will allocate an FPR for an integer mode pseudo instead of spilling
      to memory if an FPR is present in the allocno class.  It is rare that
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 0354ef3a8b3f3543800235885a942157cb081efd..2f63a9c71474c5f23c148952488a3ff86d3e2837 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -2853,9 +2853,9 @@ value that the middle-end intended.
 
 @end defmac
 
-@deftypefn {Target Hook} reg_class_t TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS (int, @var{reg_class_t})
+@deftypefn {Target Hook} reg_class_t TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS (int, @var{reg_class_t}, @var{reg_class_t})
 A target hook which can change allocno class for given pseudo from
-  allocno class calculated by IRA.
+  allocno and best class calculated by IRA.
   
   The default version of this target hook always returns given class.
 @end deftypefn
diff --git a/gcc/ira-costs.c b/gcc/ira-costs.c
index 74ee82e6d920c38c5768816eea9bc356d0f448c3..5786e1de6d671daa4f2e500fa26c9d5e9694d48e 100644
--- a/gcc/ira-costs.c
+++ b/gcc/ira-costs.c
@@ -1849,7 +1849,7 @@ find_costs_and_classes (FILE *dump_file)
 	    }
 	  if ((new_class
 	       = (reg_class) (targetm.ira_change_pseudo_allocno_class
-			      (i, regno_aclass[i]))) != regno_aclass[i])
+			      (i, regno_aclass[i], best))) != regno_aclass[i])
 	    {
 	      regno_aclass[i] = new_class;
 	      if (hard_reg_set_subset_p (reg_class_contents[new_class],
diff --git a/gcc/target.def b/gcc/target.def
index 2ec24b6794c9b349cd8b569f5dac868fd572782b..8b4105dfc71b953c5a305fa9018233fb7e6f0c43 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -4891,10 +4891,10 @@ This is currently used only by the C and C++ front ends.",
 DEFHOOK
 (ira_change_pseudo_allocno_class,
  "A target hook which can change allocno class for given pseudo from\n\
-  allocno class calculated by IRA.\n\
+  allocno and best class calculated by IRA.\n\
   \n\
   The default version of this target hook always returns given class.",
- reg_class_t, (int, reg_class_t),
+ reg_class_t, (int, reg_class_t, reg_class_t),
  default_ira_change_pseudo_allocno_class)
 
 /* Return true if we use LRA instead of reload.  */
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 47b5cfc3b8bd591c8e8f4a2a82de690d1d8b4bde..63b2b84b407829261a6c11404cbace84096fe1cc 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -148,7 +148,8 @@ extern rtx default_static_chain (const_tree, bool);
 extern void default_trampoline_init (rtx, tree, rtx);
 extern int default_return_pops_args (tree, tree, int);
 extern reg_class_t default_branch_target_register_class (void);
-extern reg_class_t default_ira_change_pseudo_allocno_class (int, reg_class_t);
+extern reg_class_t default_ira_change_pseudo_allocno_class (int, reg_class_t,
+							    reg_class_t);
 extern bool default_lra_p (void);
 extern int default_register_priority (int);
 extern bool default_register_usage_leveling_p (void);
diff --git a/gcc/targhooks.c b/gcc/targhooks.c
index dcf08631c402f25b7dcfb9f207cf02bcb622ff21..a95db48bfd164f13c37d6ef86569c4d45e19d98a 100644
--- a/gcc/targhooks.c
+++ b/gcc/targhooks.c
@@ -899,7 +899,8 @@ default_branch_target_register_class (void)
 
 reg_class_t
 default_ira_change_pseudo_allocno_class (int regno ATTRIBUTE_UNUSED,
-					 reg_class_t cl)
+					 reg_class_t cl,
+					 reg_class_t best_cl ATTRIBUTE_UNUSED)
 {
   return cl;
 }