[PowerPC] Fix pr25212

[Alan Modra]

On Sun, Dec 04, 2005 at 07:03:12PM -0800, Ian Lance Taylor wrote:
> Alan Modra <amodra@bigpond.net.au> writes:
> 
> > This patch provides a function that returns the reg class of an input
> > reload for a given piece of rtl, allowing address and memory predicates
> > to see what is happening in reload.  I use this in
> > legitimate_indexed_address_p to match addresses that have already had
> > some reloads, catching more cases than my fix for PR24997.
> > 
> > While fiddling with this patch, I noticed that INT_REG_OK_FOR_BASE_P can
> > return true for a pseudo reg that has reg_renumber[REGNO] == 0, so I
> > fixed this problem too.  Bootstrap and regression tests on powerpc-linux
> > and powerpc64-linux in progress.  OK for mainline and 4.1 assuming no
> > regressions?  (If input_reload_for doesn't seem generally useful, I can
> > move it to rs6000.c.)
> > 
> > :ADD_PATCH target, middle-end:
> > 
> > 	PR target/25212
> > 	* reload.c (input_reload_for): New function.
> > 	* reload.h (input_reload_for): Prototype.
> > 	* config/rs6000/rs6000.c (is_base_index): New function.
> > 	(legitimate_indexed_address_p): Use is_base_index.
> > 	* config/rs6000/rs6000.h (INT_REG_OK_FOR_INDEX_P): Simplify.
> > 	(INT_REG_OK_FOR_BASE_P): Correct.
> 
> This is only going to work if you know for sure that during any call
> to memory_address_p or strict_memory_address_p during reload the
> n_reloads and rld variables will be initialized properly for the
> instruction containing the address which is being checked.  Do we
> really know that?  For example, find_reloads will reset n_reloads to 0
> at the start of the function.  It can then call memory_address_p (via,
> e.g., find_reloads_toplev) before n_reloads has been restored to its
> old value.  But some of the addresses in the instruction may be being
> reloaded, due to a previous call to find_reloads.

Won't reloads be recalculated on each reload pass?  From what I can see
in reload(), calculate_needs_all_insns() starts from scratch each pass.
It generates an insn chain with reloads using copy_reloads(), but this
is destroyed if another pass is needed.  The reload obstack is freed.
Some rtl transformations are kept between passes, but that's fine since
we see those.  (The fact that reloads aren't kept between passes is why
legitimize_reload_address must recognize any transformations it makes,
and call push_reload as appropriate on the transformed rtl.)

Note that I don't need or want the full set of input reloads for the
instruction, just those that have been found up to a particular point.

> So it seems to me that input_reload_for is not reliable.

Hmm, there is a small problem in that n_reloads isn't cleared, so we
could be looking at stale data.  For example, eliminate_regs_in_insn
could conceivably be making insn changes that need to be verified by
memory_address_p, confusing a predicate that uses input_reload_for.
Easily fixed with this additional patch.

       * reload1.c (calculate_needs_all_insns): Clear n_reloads.
       (reload_as_needed): Likewise.

Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	(revision 108041)
+++ gcc/reload1.c	(working copy)
@@ -1531,6 +1531,9 @@ calculate_needs_all_insns (int global)
 	      copy_reloads (chain);
 	      *pprev_reload = chain;
 	      pprev_reload = &chain->next_need_reload;
+	      /* Zero n_reloads so that input_reload_for doesn't look at
+		 stale data.  */
+	      n_reloads = 0;
 	    }
 	}
     }
@@ -4119,6 +4122,10 @@ reload_as_needed (int live_known)
 	AND_COMPL_HARD_REG_SET (reg_reloaded_valid, call_used_reg_set);
 	AND_COMPL_HARD_REG_SET (reg_reloaded_valid, reg_reloaded_call_part_clobbered);
 	}
+
+      /* Zero n_reloads so that input_reload_for doesn't look at
+	 stale data.  */
+      n_reloads = 0;
     }
 
   /* Clean up.  */


> If LEGITIMIZE_RELOAD_ADDRESS can transform the address, then it is
> clearly useful to know how that has been done.  But I don't think the
> approach you are proposing will work.

I tried the approach you suggested on irc, namely, relaxing the
predicate to accept invalid addresses on the grounds that MEMs will have
gone through find_reloads_address so we can assume that reload has fixed
them.  This failed with

/src/gcc-current/gcc/libgcov.c:532: error: insn does not satisfy its constraints:
(insn:HI 658 1751 1752 69 /src/gcc-current/gcc/libgcov.c:368 (set (reg:DI 0 0 [403])
        (zero_extend:DI (mem/s:SI (plus:DI (plus:DI (reg/v/f:DI 21 21 [orig:135 fi_ptr.728 ] [135])
                        (const_int 8 [0x8]))
                    (reg:DI 11 11 [401])) [3 <variable>.n_ctrs S4 A32]))) 14 {*zero_extendsidi2_internal1} (insn_list:REG_DEP_TRUE 656 (nil))
    (nil))

because that particular mem *hasn't* gone through find_reloads_address
prior to the point that constraints are matched.  I guess this is a
bug, and reload should look inside zero_extend and suchlike..
However, it illustrates that guessing what reload will do is rather
dangerous.  I still rather like the idea of using rld[] to see what
reload will do.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre