From ef704984acf483441bebcbf780c691e05ad6ba65 Mon Sep 17 00:00:00 2001 From: wilco Date: Tue, 2 Feb 2016 17:12:56 +0000 Subject: [PATCH] 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 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 --- gcc/ChangeLog | 13 +++++++++++++ gcc/config/aarch64/aarch64.c | 21 +++++++++++++++++---- gcc/config/mips/mips.c | 3 ++- gcc/doc/tm.texi | 4 ++-- gcc/ira-costs.c | 2 +- gcc/target.def | 4 ++-- gcc/targhooks.c | 3 ++- gcc/targhooks.h | 3 ++- 8 files changed, 41 insertions(+), 12 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 57a7df7..0e72ed1 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,18 @@ 2016-02-02 Wilco Dijkstra + * 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 + * 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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 --- 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); -- 2.7.4