(this came up for m68k vs. LRA, but is a generic problem)

Regrename wants to use new registers for certain def-use chains.
For validity of replacements it needs to check that the selected
candidates are unused up to then.  That's done in check_new_reg_p.
But if it so happens that the new register needs more hardregs
than the old register (which happens if the target allows inter-bank
moves and the mode is something like a DFmode that needs to be placed
into a SImode reg-pair), then check_new_reg_p only checks the
first of those registers for free-ness.

This is caused by that function looking up the number of necessary
hardregs only in terms of the old hardreg number.  It of course needs
to do that in terms of the new candidate regnumber.  The symptom is that
regrename sometimes clobbers the higher numbered registers of such a
regrename target pair.  This patch fixes that problem.

(In the particular case of the bug report it was LRA that left over a
inter-bank move instruction that triggers regrename, ultimately causing
the mis-compile.  Reload didn't do that, but in general we of course
can't rely on such moves not happening if the target allows them.)

This also shows a general confusion in that function and the target hook
interface here:

  for (i = nregs - 1; i >= 0; --)
    ...
    || ! HARD_REGNO_RENAME_OK (reg + i, new_reg + i))

it uses nregs in a way that requires it to be the same between old and
new register.  The problem is that the target hook only gets register
numbers, when it instead should get a mode and register numbers and
would be called only for the first but not for subsequent registers.
I've looked at a number of definitions of that target hook and I think
that this is currently harmless in the sense that it would merely rule
out some potential reg-renames that would in fact be okay to do.  So I'm
not changing the target hook interface here and hence that problem
remains unfixed.

        PR rtl-optimization/116650
        * regrename.cc (check_new_reg_p): Calculate nregs in terms of
        the new candidate register.
---

Regstrapped on x86-64-linux, okay for trunk?


Ciao,
Michael.

---
 gcc/regrename.cc | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/gcc/regrename.cc b/gcc/regrename.cc
index 054e601740b..22668d7bf57 100644
--- a/gcc/regrename.cc
+++ b/gcc/regrename.cc
@@ -324,10 +324,27 @@ static bool
 check_new_reg_p (int reg ATTRIBUTE_UNUSED, int new_reg,
                 class du_head *this_head, HARD_REG_SET this_unavailable)
 {
-  int nregs = this_head->nregs;
+  int nregs = 1;
   int i;
   struct du_chain *tmp;
 
+  /* See whether new_reg accepts all modes that occur in
+     definition and uses and record the number of regs it would take.  */
+  for (tmp = this_head->first; tmp; tmp = tmp->next_use)
+    {
+      int n;
+      /* Completely ignore DEBUG_INSNs, otherwise we can get
+        -fcompare-debug failures.  */
+      if (DEBUG_INSN_P (tmp->insn))
+       continue;
+
+      if (!targetm.hard_regno_mode_ok (new_reg, GET_MODE (*tmp->loc)))
+       return false;
+      n = hard_regno_nregs (new_reg, GET_MODE (*tmp->loc));
+      if (n > nregs)
+       nregs = n;
+    }
+
   for (i = nregs - 1; i >= 0; --i)
     if (TEST_HARD_REG_BIT (this_unavailable, new_reg + i)
        || fixed_regs[new_reg + i]
@@ -348,14 +365,10 @@ check_new_reg_p (int reg ATTRIBUTE_UNUSED, int new_reg,
      definition and uses.  */
   for (tmp = this_head->first; tmp; tmp = tmp->next_use)
     {
-      /* Completely ignore DEBUG_INSNs, otherwise we can get
-        -fcompare-debug failures.  */
       if (DEBUG_INSN_P (tmp->insn))
        continue;
 
-      if (!targetm.hard_regno_mode_ok (new_reg, GET_MODE (*tmp->loc))
-         || call_clobbered_in_chain_p (this_head, GET_MODE (*tmp->loc),
-                                       new_reg))
+      if (call_clobbered_in_chain_p (this_head, GET_MODE (*tmp->loc), new_reg))
        return false;
     }
 
-- 
2.39.1

Reply via email to