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 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 to force to GENERAL_REGS or FP_REGS).

Previously this might happen:

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.

2016-02-02  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.

git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@233084 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 57a7df7..0e72ed1 100644 (file)
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,18 @@
 2016-02-02  Wilco Dijkstra  <wdijkstr@arm.com>
 
+       * 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.
+
+2016-02-02  Wilco Dijkstra  <wdijkstr@arm.com>
+
        * config/aarch64/aarch64.c
        (TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS): New define.
        (aarch64_ira_change_pseudo_allocno_class): New function.

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e7cfcb6..3ff12ff 100644 (file)
--- 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 9ec2dcb..697abc2 100644 (file)
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19916,7 +19916,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 2392691..faf8bcb 100644 (file)
--- 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 2b736c1..f3d31e1 100644 (file)
--- a/gcc/ira-costs.c
+++ b/gcc/ira-costs.c
@@ -1858,7 +1858,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 fa0af67..f139ef9 100644 (file)
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -4874,10 +4874,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.c b/gcc/targhooks.c
index 8a162a1..fa6a43d 100644 (file)
--- 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;
 }

diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index 7ab647f..7687c39 100644 (file)
--- 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);