From 20d70cd2719815d9ea853314775ae5787648ece5 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Thu, 9 May 2019 08:37:26 +0930 Subject: [PATCH] [RS6000] PR89271, gcc.target/powerpc/vsx-simode2.c This patch makes a number of corrections to rs6000_register_move_cost, adds a new register union class, GEN_OR_VSX_REGS, and adjusts insn alternative costs to suit. The patch initially just corrected register move cost when direct moves are available, but that resulted in regressions. Inspection of those regressions showed ALL_REGS being used as the register allocno class, which isn't ideal. gcc/doc/tm.texi says: "You should define a class for the union of two classes whenever some instruction allows both classes". Thus, define GEN_OR_VSX_REGS for the register allocator. (IRA wants to use the union of two register classes when the costs of the classes are below memory cost, which happens more often with the low direct move cost.) As per https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89271#c11 we ought to be returning the minimal cost for union classes. That can be done by rs6000_register_move_cost testing for vsx first, where the number of regs for a given mode might be smaller than the same mode in gprs, and changing the LINK_OR_CTR_REGS case to exclude SPEC_OR_GEN_REGS and NON_FLOAT_REGS. I removed the VECTOR_MEM_VSX_P test since that leads to silly results for scalar mode moves between altivec and float when TARGET_VSX. eg. rs6000_register_move_cost:, ret=2, mode=DF, from=FLOAT_REGS, to=FLOAT_REGS rs6000_register_move_cost:, ret=16, mode=DF, from=FLOAT_REGS, to=ALTIVEC_REGS rs6000_register_move_cost:, ret=2, mode=DF, from=FLOAT_REGS, to=VSX_REGS The patch also fixes wrong results for moves within and between any of the non-gpr, non-vsx special reg classes. The comment about "moving between two similar registers is just one instruction" is false. We can't move lr to ctr directly, for example. I believe the intent of the "reg_classes_intersect_p (to, from)" was to cover moves within float or altivec, so I moved that test inside the code handling vsx, and made sure the intersection wasn't anything besides vsx by masking off everything else. Masking isn't strictly necessary at the moment, but would be if we create a GEN_OR_ALTIVEC_REGS class some time in the future. TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS is needed for rs6000 in order to fix the 20% cactus_adm spec regression when using GEN_OR_VSX_REGS as an allocno class. It is similar to the aarch64 version but without any selection by regno mode if the best class is a union class. PR target/89271 * config/rs6000/rs6000.h (enum reg_class, REG_CLASS_NAMES), (REG_CLASS_CONTENTS): Add GEN_OR_VSX_REGS class. * config/rs6000/rs6000.c (rs6000_register_move_cost): Correct cost for general <-> vsx when direct moves are available. Cost union classes at minimal cost for any reg in the class. Correct calculation for moves between vsx, float, and altivec. Don't return a low cost for moves between special regs. Don't use hard coded register numbers. (TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS): Define. (rs6000_ira_change_pseudo_allocno_class): New function. * config/rs6000/rs6000.md (movsi_internal1, mov_internal), (movdi_internal32, movdi_internal64): Remove '*' from vsx register alternatives. (movsi_internal1): Don't disparage vector alternatives. (mov_internal): Likewise, excepting alternative that will be split. * config/rs6000/vsx.md (vsx_splat__reg): Don't disparage we <- b alternative. From-SVN: r271022 --- gcc/ChangeLog | 22 +++++++ gcc/config/rs6000/rs6000.c | 127 +++++++++++++++++++++++++++++------- gcc/config/rs6000/rs6000.h | 4 ++ gcc/config/rs6000/rs6000.md | 26 ++++---- gcc/config/rs6000/vsx.md | 2 +- 5 files changed, 145 insertions(+), 36 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 1fe31a7d40e..37447f854c0 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,25 @@ +2019-05-09 Alan Modra + + PR target/89271 + * config/rs6000/rs6000.h (enum reg_class, REG_CLASS_NAMES), + (REG_CLASS_CONTENTS): Add GEN_OR_VSX_REGS class. + * config/rs6000/rs6000.c (rs6000_register_move_cost): Correct + cost for general <-> vsx when direct moves are available. + Cost union classes at minimal cost for any reg in the class. + Correct calculation for moves between vsx, float, and altivec. + Don't return a low cost for moves between special regs. Don't + use hard coded register numbers. + (TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS): Define. + (rs6000_ira_change_pseudo_allocno_class): New function. + * config/rs6000/rs6000.md (movsi_internal1, mov_internal), + (movdi_internal32, movdi_internal64): Remove '*' from vsx register + alternatives. + (movsi_internal1): Don't disparage vector alternatives. + (mov_internal): Likewise, excepting alternative that + will be split. + * config/rs6000/vsx.md (vsx_splat__reg): Don't disparage + we <- b alternative. + 2019-05-08 Jakub Jelinek PR c++/59813 diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 5d5765d89b2..3d1af9f1bc9 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -1729,6 +1729,9 @@ static const struct attribute_spec rs6000_attribute_table[] = #define TARGET_REGISTER_MOVE_COST rs6000_register_move_cost #undef TARGET_MEMORY_MOVE_COST #define TARGET_MEMORY_MOVE_COST rs6000_memory_move_cost +#undef TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS +#define TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS \ + rs6000_ira_change_pseudo_allocno_class #undef TARGET_CANNOT_COPY_INSN_P #define TARGET_CANNOT_COPY_INSN_P rs6000_cannot_copy_insn_p #undef TARGET_RTX_COSTS @@ -34648,22 +34651,54 @@ rs6000_register_move_cost (machine_mode mode, reg_class_t from, reg_class_t to) { int ret; + reg_class_t rclass; if (TARGET_DEBUG_COST) dbg_cost_ctrl++; + /* If we have VSX, we can easily move between FPR or Altivec registers, + otherwise we can only easily move within classes. + Do this first so we give best-case answers for union classes + containing both gprs and vsx regs. */ + HARD_REG_SET to_vsx, from_vsx; + COPY_HARD_REG_SET (to_vsx, reg_class_contents[to]); + AND_HARD_REG_SET (to_vsx, reg_class_contents[VSX_REGS]); + COPY_HARD_REG_SET (from_vsx, reg_class_contents[from]); + AND_HARD_REG_SET (from_vsx, reg_class_contents[VSX_REGS]); + if (!hard_reg_set_empty_p (to_vsx) + && !hard_reg_set_empty_p (from_vsx) + && (TARGET_VSX + || hard_reg_set_intersect_p (to_vsx, from_vsx))) + { + int reg = FIRST_FPR_REGNO; + if (TARGET_VSX + || (TEST_HARD_REG_BIT (to_vsx, FIRST_ALTIVEC_REGNO) + && TEST_HARD_REG_BIT (from_vsx, FIRST_ALTIVEC_REGNO))) + reg = FIRST_ALTIVEC_REGNO; + ret = 2 * hard_regno_nregs (reg, mode); + } + /* Moves from/to GENERAL_REGS. */ - if (reg_classes_intersect_p (to, GENERAL_REGS) - || reg_classes_intersect_p (from, GENERAL_REGS)) + else if ((rclass = from, reg_classes_intersect_p (to, GENERAL_REGS)) + || (rclass = to, reg_classes_intersect_p (from, GENERAL_REGS))) { - reg_class_t rclass = from; - - if (! reg_classes_intersect_p (to, GENERAL_REGS)) - rclass = to; - if (rclass == FLOAT_REGS || rclass == ALTIVEC_REGS || rclass == VSX_REGS) - ret = (rs6000_memory_move_cost (mode, rclass, false) - + rs6000_memory_move_cost (mode, GENERAL_REGS, false)); + { + if (TARGET_DIRECT_MOVE) + { + if (rs6000_tune == PROCESSOR_POWER9) + ret = 2 * hard_regno_nregs (FIRST_GPR_REGNO, mode); + else + ret = 4 * hard_regno_nregs (FIRST_GPR_REGNO, mode); + /* SFmode requires a conversion when moving between gprs + and vsx. */ + if (mode == SFmode) + ret += 2; + } + else + ret = (rs6000_memory_move_cost (mode, rclass, false) + + rs6000_memory_move_cost (mode, GENERAL_REGS, false)); + } /* It's more expensive to move CR_REGS than CR0_REGS because of the shift. */ @@ -34676,24 +34711,14 @@ rs6000_register_move_cost (machine_mode mode, || rs6000_tune == PROCESSOR_POWER7 || rs6000_tune == PROCESSOR_POWER8 || rs6000_tune == PROCESSOR_POWER9) - && reg_classes_intersect_p (rclass, LINK_OR_CTR_REGS)) - ret = 6 * hard_regno_nregs (0, mode); + && reg_class_subset_p (rclass, SPECIAL_REGS)) + ret = 6 * hard_regno_nregs (FIRST_GPR_REGNO, mode); else /* A move will cost one instruction per GPR moved. */ - ret = 2 * hard_regno_nregs (0, mode); + ret = 2 * hard_regno_nregs (FIRST_GPR_REGNO, mode); } - /* If we have VSX, we can easily move between FPR or Altivec registers. */ - else if (VECTOR_MEM_VSX_P (mode) - && reg_classes_intersect_p (to, VSX_REGS) - && reg_classes_intersect_p (from, VSX_REGS)) - ret = 2 * hard_regno_nregs (FIRST_FPR_REGNO, mode); - - /* Moving between two similar registers is just one instruction. */ - else if (reg_classes_intersect_p (to, from)) - ret = (FLOAT128_2REG_P (mode)) ? 4 : 2; - /* Everything else has to go through GENERAL_REGS. */ else ret = (rs6000_register_move_cost (mode, GENERAL_REGS, to) @@ -34746,6 +34771,64 @@ rs6000_memory_move_cost (machine_mode mode, reg_class_t rclass, return ret; } +/* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS. + + The register allocator chooses GEN_OR_VSX_REGS for the allocno + class if GENERAL_REGS and VSX_REGS cost is lower than the memory + cost. This happens a lot when TARGET_DIRECT_MOVE makes the register + move cost between GENERAL_REGS and VSX_REGS low. + + It might seem reasonable to use a union class. After all, if usage + of vsr is low and gpr high, it might make sense to spill gpr to vsr + rather than memory. However, in cases where register pressure of + both is high, like the cactus_adm spec test, allowing + GEN_OR_VSX_REGS as the allocno class results in bad decisions in + the first scheduling pass. This is partly due to an allocno of + GEN_OR_VSX_REGS wrongly contributing to the GENERAL_REGS pressure + class, which gives too high a pressure for GENERAL_REGS and too low + for VSX_REGS. So, force a choice of the subclass here. + + The best class is also the union if GENERAL_REGS and VSX_REGS have + the same cost. In that case we do use GEN_OR_VSX_REGS as the + allocno class, since trying to narrow down the class by regno mode + is prone to error. For example, SImode is allowed in VSX regs and + in some cases (eg. gcc.target/powerpc/p9-xxbr-3.c do_bswap32_vect) + it would be wrong to choose an allocno of GENERAL_REGS based on + SImode. */ + +static reg_class_t +rs6000_ira_change_pseudo_allocno_class (int regno ATTRIBUTE_UNUSED, + reg_class_t allocno_class, + reg_class_t best_class) +{ + switch (allocno_class) + { + case GEN_OR_VSX_REGS: + /* best_class must be a subset of allocno_class. */ + gcc_checking_assert (best_class == GEN_OR_VSX_REGS + || best_class == GEN_OR_FLOAT_REGS + || best_class == VSX_REGS + || best_class == ALTIVEC_REGS + || best_class == FLOAT_REGS + || best_class == GENERAL_REGS + || best_class == BASE_REGS); + /* Use best_class but choose wider classes when copying from the + wider class to best_class is cheap. This mimics IRA choice + of allocno class. */ + if (best_class == BASE_REGS) + return GENERAL_REGS; + if (TARGET_VSX + && (best_class == FLOAT_REGS || best_class == ALTIVEC_REGS)) + return VSX_REGS; + return best_class; + + default: + break; + } + + return allocno_class; +} + /* Returns a code for a target-specific builtin that implements reciprocal of the function, or NULL_TREE if not available. */ diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index 14a9e199bc8..68ef491ac0c 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -1141,6 +1141,7 @@ enum reg_class VRSAVE_REGS, VSCR_REGS, GEN_OR_FLOAT_REGS, + GEN_OR_VSX_REGS, LINK_REGS, CTR_REGS, LINK_OR_CTR_REGS, @@ -1169,6 +1170,7 @@ enum reg_class "VRSAVE_REGS", \ "VSCR_REGS", \ "GEN_OR_FLOAT_REGS", \ + "GEN_OR_VSX_REGS", \ "LINK_REGS", \ "CTR_REGS", \ "LINK_OR_CTR_REGS", \ @@ -1205,6 +1207,8 @@ enum reg_class { 0x00000000, 0x00000000, 0x00000000, 0x00002000 }, \ /* GEN_OR_FLOAT_REGS. */ \ { 0xffffffff, 0xffffffff, 0x00000000, 0x00004008 }, \ + /* GEN_OR_VSX_REGS. */ \ + { 0xffffffff, 0xffffffff, 0xffffffff, 0x00004008 }, \ /* LINK_REGS. */ \ { 0x00000000, 0x00000000, 0x00000000, 0x00000001 }, \ /* CTR_REGS. */ \ diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 411d7f0d352..8da7aba4080 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -6830,10 +6830,10 @@ ;; MF%1 MT%0 NOP (define_insn "*movsi_internal1" [(set (match_operand:SI 0 "nonimmediate_operand" - "=r, r, r, ?*wI, ?*wH, - m, ?Z, ?Z, r, r, - r, ?*wIwH, ?*wJwK, ?*wJwK, ?*wu, - ?*wJwK, ?*wH, ?*wK, ?*wIwH, ?r, + "=r, r, r, wI, wH, + m, Z, Z, r, r, + r, wIwH, wJwK, wJwK, wu, + wJwK, wH, wK, wIwH, r, r, *h, *h") (match_operand:SI 1 "input_operand" @@ -7104,13 +7104,13 @@ ;; MTVSRWZ MF%1 MT%1 NOP (define_insn "*mov_internal" [(set (match_operand:QHI 0 "nonimmediate_operand" - "=r, r, ?*wJwK, m, Z, r, - ?*wJwK, ?*wJwK, ?*wJwK, ?*wK, ?*wK, r, - ?*wJwK, r, *c*l, *h") + "=r, r, wJwK, m, Z, r, + wJwK, wJwK, wJwK, wK, ?wK, r, + wJwK, r, *c*l, *h") (match_operand:QHI 1 "input_operand" "r, m, Z, r, wJwK, i, - wJwK, O, wM, wB, wS, ?*wJwK, + wJwK, O, wM, wB, wS, wJwK, r, *h, r, 0"))] "gpc_reg_operand (operands[0], mode) @@ -8671,8 +8671,8 @@ [(set (match_operand:DI 0 "nonimmediate_operand" "=Y, r, r, m, ^d, ^d, r, wY, Z, ^wb, $wv, ^wi, - *wo, *wo, *wv, *wi, *wi, *wv, - *wv") + wo, wo, wv, wi, *i, wv, + wv") (match_operand:DI 1 "input_operand" "r, Y, r, ^d, m, ^d, @@ -8751,9 +8751,9 @@ [(set (match_operand:DI 0 "nonimmediate_operand" "=YZ, r, r, r, r, r, m, ^d, ^d, wY, Z, $wb, - $wv, ^wi, *wo, *wo, *wv, *wi, - *wi, *wv, *wv, r, *h, *h, - ?*r, ?*wg, ?*r, ?*wj") + $wv, ^wi, wo, wo, wv, wi, + wi, wv, wv, r, *h, *h, + ?r, ?wg, ?r, ?wj") (match_operand:DI 1 "input_operand" "r, YZ, r, I, L, nF, diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md index 607c0cd33f2..80434d10247 100644 --- a/gcc/config/rs6000/vsx.md +++ b/gcc/config/rs6000/vsx.md @@ -4106,7 +4106,7 @@ }) (define_insn "vsx_splat__reg" - [(set (match_operand:VSX_D 0 "vsx_register_operand" "=,?we") + [(set (match_operand:VSX_D 0 "vsx_register_operand" "=,we") (vec_duplicate:VSX_D (match_operand: 1 "gpc_reg_operand" ",b")))] "VECTOR_MEM_VSX_P (mode)" -- 2.30.2