reload doesn't compensate for reloaded POST_INC in inherited address

[Alexandre Oliva <aoliva at redhat dot com>]

The mn10300 port has this trait that not all registers can be used in
indirect addressing modes.  I got a testcase that exposes a problem in
reload in which it fails to handle post-increment addresses that have
to be reloaded but whose values are already available in other registers.

The lreg dump contained instructions like this:

[...]
(set (reg pseudo) (...))
(set (...) (mem (reg pseudo)))
[...]
(set (...) (mem (post_inc (reg pseudo))))
[...]
(set (...) (mem (reg pseudo)))
[...]
(set (...) (mem (post_inc (reg pseudo))))
[...]
(set (...) (mem (reg pseudo)))
[...]

greg assigned pseudo to d3, that, being a data register, cannot be
used in indirect address modes (*), so we push a reload for every
occurrence of the now-(post_inc (reg d3)) (as well as input reloads
for the enclosing MEMs), and for the occurrences of (reg d3) within
MEMs.

reload then turns the sequence into:

(set (reg d3) (...))
(set (reg a3) (reg d3))
(set (...) (mem (reg a3)))
[...]
(set (...) (mem (post_inc (reg a3))))
[...]
(set (...) (mem (reg a3)))
[...] ;; uses a3 in some other reloads
(set (reg a1) (reg d3))
(set (reg d3) (plus (reg d3) (const_int 4)))
(set (...) (mem (post_inc (reg a1))))
[...]
(set (...) (mem (reg a1)))

Note that, in the first occurrence of (post_inc (reg d3)), its
reload_input had been replaced with (reg a3) (reload_reg_rtx), because
a3 already contained the correct value, whereas in the second case, no
address register held a copy of d3.

The problem is that, even though d3 was replaced with a3 within the
post_inc, no code to compensate this was added, so when we got to the
second post_inc, we used a value in d3 that was out-of-date.

The only place I found that would handle this kind of
auto_inc_dec-compensation was in inc_for_reload(), called from
emit_input_reload_insns(), but at such places, it would only handle
cases whose input reloads were MEMs.  In this case, not only the input
is a REG, but also the output of the reload was not a MEM, but rather
a POST_INC of a REG.  The patch below adjusts the code so as to comply
with this case.

To give some more context than the patch, here's the chunk of code
I've modified:

  rtx old = (rl->in
	     && (GET_CODE (rl->in) == MEM
#ifdef AUTO_INC_DEC
		 /* If the reload is an auto_inc_dec (as opposed to a
		    MEM containing one), as is the case when the
		    address of the enclosing MEM is not legitimate
		    (say, a data register on mn10300) and needs a
		    reload, we have to make sure we generate code to
		    apply the increment to the original register.  */
		 || (rl->in_reg && rl->in_reg == rl->out
		     && (GET_CODE (rl->in_reg) == PRE_INC
			 || GET_CODE (rl->in_reg) == PRE_DEC
			 || GET_CODE (rl->in_reg) == POST_INC
			 || GET_CODE (rl->in_reg) == POST_DEC))
#endif
		 ) ? rl->in_reg : rl->in);

  if (old != 0
      /* AUTO_INC reloads need to be handled even if inherited.  We got an
	 AUTO_INC reload if reload_out is set but reload_out_reg isn't.  */
      && (! reload_inherited[j] || (rl->out && ! rl->out_reg))
      && ! rtx_equal_p (rl->reg_rtx, old)
      && rl->reg_rtx != 0)
    emit_input_reload_insns (chain, rld + j, old, j);

`old' is not used in the rest of do_input_reloads(), and we actually
need it to contain the INC/DEC rtx, instead of the reused reg, so I
think the patch is correct.  Regression testing on mn10300-elf -mam33
has revealed no regressions, and I bootstrapped on athlon-pc-linux-gnu
just in case (the patch couldn't possibly affect it, since
AUTO_INC_DEC is not set, but I hadn't realized that when I started the
bootstrap; d'oh!).  I intend to give this patch a spin on mn10300-elf
and sh-elf if I manage to get them to build.  My last attempt to build
mn10300-elf failed badly, and I haven't tried sh-elf on recent trees
for a while :-(  I've regression tested an old tree, for which I had
baseline test results.

Anyway, does this patch look ok to install?

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* reload1.c (do_input_reload): Handle auto_inc_dec reloads that
	are not reloads of their MEMs.

Index: gcc/reload1.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/reload1.c,v
retrieving revision 1.346
diff -u -p -r1.346 reload1.c
--- gcc/reload1.c 17 Jun 2002 11:33:20 -0000 1.346
+++ gcc/reload1.c 4 Jul 2002 05:29:35 -0000
@@ -6852,8 +6852,22 @@ do_input_reload (chain, rl, j)
 {
   int expect_occurrences = 1;
   rtx insn = chain->insn;
-  rtx old = (rl->in && GET_CODE (rl->in) == MEM
-	     ? rl->in_reg : rl->in);
+  rtx old = (rl->in
+	     && (GET_CODE (rl->in) == MEM
+#ifdef AUTO_INC_DEC
+		 /* If the reload is an auto_inc_dec (as opposed to a
+		    MEM containing one), as is the case when the
+		    address of the enclosing MEM is not legitimate
+		    (say, a data register on mn10300) and needs a
+		    reload, we have to make sure we generate code to
+		    apply the increment to the original register.  */
+		 || (rl->in_reg && rl->in_reg == rl->out
+		     && (GET_CODE (rl->in_reg) == PRE_INC
+			 || GET_CODE (rl->in_reg) == PRE_DEC
+			 || GET_CODE (rl->in_reg) == POST_INC
+			 || GET_CODE (rl->in_reg) == POST_DEC))
+#endif
+		 ) ? rl->in_reg : rl->in);
 
   if (old != 0
       /* AUTO_INC reloads need to be handled even if inherited.  We got an

* to be fair, on AM33, it can, and AM30 does not support
post-increment, so the problem could be theoretically be worked around
by changing the register classes that we could choose to reload
addresses, but using address registers still preferable, and changing
this has been difficult in the past.  Unfortunately, I don't remember
the details.

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist                Professional serial bug killer