From 0b3839a4b267858948f0d2fe9ec600129d5d14b4 Mon Sep 17 00:00:00 2001 From: Richard Sandiford Date: Sun, 7 Jul 2019 09:49:24 +0000 Subject: [PATCH] PR91068: Fix MIPS fallout from IRA matched operand changes PR91068 is a case in which we have (ignoring non-LRA alternatives): [(set (match_operand:SI 0 "register_operand" "=l,d?") (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d") (match_operand:SI 2 "register_operand" "d,d")) (match_operand:SI 3 "register_operand" "0,d"))) (clobber (match_scratch:SI 4 "=X,l")) (clobber (match_scratch:SI 5 "=X,&d"))] where the first alternative is one instruction but the second is two. This is very similar to the case that my recent IRA patches were supposed to help. The crucial difference is that the cheap alternative requires a single-register class while the expensive alternative uses general registers. This makes a difference when one of operand 0 or 3 can naturally be allocated to LO but the other can't. If IRA makes that allocation, both alternatives require one reload of equal cost and so the first alternative clearly wins. However, if we say that tying operands 0 and 3 saves the cost of a full move, then all other things being equal, IRA will prefer to allocate both registers to the same GPR. The registers will then naturally fit the second alternative. This has a more drastic effect in the MIPS case than it should because using the GPR alternative is much more expensive there than it appears to the RA. But that's really a separate problem and something we were able to live with before my IRA patch. What makes tying less useful here is the fact that the tied register is a single-register class. I think in those circumstances it's better not to use tied operands at all and instead use "l" for the inputs. Allocating the input to LO, and allocating the output to LO, then depend naturally on class costs. If we decide to allocate at least one of them to LO, we'll use the cheap alternative, otherwise we'll (correctly) use the expensive alternative. This effectively restores the situation before my IRA patch, but this time making the preference on the input register more explicit. I originally wrote the patterns in the early days of IRA, and certainly well before LRA. I think they were largely influened by reload rather than RA proper (see the comment above *mul_acc_si, which is all about the reload behaviour). LRA copes with the two-"l" case just fine. The patch may well cause problems for -mno-lra, but I think we should cull that option anyway. 2019-07-07 Richard Sandiford gcc/ PR target/91068 * config/mips/mips.md (*mul_acc_si, *mul_acc_si_r3900, *macc) (*msac, *msac_using_macc, *mul_sub_si): Use "l" for input operands instead of matching them to "l" output operands. From-SVN: r273175 --- gcc/ChangeLog | 7 +++++++ gcc/config/mips/mips.md | 12 ++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 13622c8abbe..802cdb8e91d 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2019-07-07 Richard Sandiford + + PR target/91068 + * config/mips/mips.md (*mul_acc_si, *mul_acc_si_r3900, *macc) + (*msac, *msac_using_macc, *mul_sub_si): Use "l" for input operands + instead of matching them to "l" output operands. + 2019-07-07 Richard Sandiford * config/mips/mips.c (mips_split_move): Zero-initialize addr diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md index d260cf9170c..e17b1d522f0 100644 --- a/gcc/config/mips/mips.md +++ b/gcc/config/mips/mips.md @@ -1749,7 +1749,7 @@ [(set (match_operand:SI 0 "register_operand" "=l*?*?,l,d?") (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d,d") (match_operand:SI 2 "register_operand" "d,d,d")) - (match_operand:SI 3 "register_operand" "0,0,d"))) + (match_operand:SI 3 "register_operand" "l,l,d"))) (clobber (match_scratch:SI 4 "=X,X,l")) (clobber (match_scratch:SI 5 "=X,X,&d"))] "GENERATE_MADD_MSUB && !TARGET_MIPS16" @@ -1778,7 +1778,7 @@ [(set (match_operand:SI 0 "register_operand" "=l*?*?,l,d*?,d?") (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d,d,d") (match_operand:SI 2 "register_operand" "d,d,d,d")) - (match_operand:SI 3 "register_operand" "0,0,l,d"))) + (match_operand:SI 3 "register_operand" "l,l,l,d"))) (clobber (match_scratch:SI 4 "=X,X,3,l")) (clobber (match_scratch:SI 5 "=X,X,X,&d"))] "TARGET_MIPS3900 && !TARGET_MIPS16" @@ -1822,7 +1822,7 @@ [(set (match_operand:SI 0 "register_operand" "=l,d") (plus:SI (mult:SI (match_operand:SI 1 "register_operand" "d,d") (match_operand:SI 2 "register_operand" "d,d")) - (match_operand:SI 3 "register_operand" "0,l"))) + (match_operand:SI 3 "register_operand" "l,l"))) (clobber (match_scratch:SI 4 "=X,3"))] "ISA_HAS_MACC" { @@ -1842,7 +1842,7 @@ (define_insn "*msac" [(set (match_operand:SI 0 "register_operand" "=l,d") - (minus:SI (match_operand:SI 1 "register_operand" "0,l") + (minus:SI (match_operand:SI 1 "register_operand" "l,l") (mult:SI (match_operand:SI 2 "register_operand" "d,d") (match_operand:SI 3 "register_operand" "d,d")))) (clobber (match_scratch:SI 4 "=X,1"))] @@ -1862,7 +1862,7 @@ ;; An msac-like instruction implemented using negation and a macc. (define_insn_and_split "*msac_using_macc" [(set (match_operand:SI 0 "register_operand" "=l,d") - (minus:SI (match_operand:SI 1 "register_operand" "0,l") + (minus:SI (match_operand:SI 1 "register_operand" "l,l") (mult:SI (match_operand:SI 2 "register_operand" "d,d") (match_operand:SI 3 "register_operand" "d,d")))) (clobber (match_scratch:SI 4 "=X,1")) @@ -2005,7 +2005,7 @@ ;; See the comment above *mul_add_si for details. (define_insn "*mul_sub_si" [(set (match_operand:SI 0 "register_operand" "=l*?*?,l,d?") - (minus:SI (match_operand:SI 1 "register_operand" "0,0,d") + (minus:SI (match_operand:SI 1 "register_operand" "l,l,d") (mult:SI (match_operand:SI 2 "register_operand" "d,d,d") (match_operand:SI 3 "register_operand" "d,d,d")))) (clobber (match_scratch:SI 4 "=X,X,l")) -- 2.30.2