[patch] Fix PR rtl-optimization/54290

[Eric Botcazou <ebotcazou at adacore dot com>]

Hi,

this is a regression present on the 4.6 branch at -O2 for the SPARC, but the 
underlying issue is presumably latent everywhere.  It's reload inheritance so 
the opinion of reload specialists is welcome.

We have a couple of insns with 2 reloads each:

Reloads for insn # 84
Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 30 %fp)
                                                    (const_int -6140 
[0xffffffffffffe804]))
	GENERAL_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 2), can't combine
	reload_in_reg: (plus:SI (reg/f:SI 30 %fp)
                                                    (const_int -6140 
[0xffffffffffffe804]))
	reload_reg_rtx: (reg:SI 11 %o3)
Reload 1: reload_in (SI) = (reg:SI 40 %f8 [orig:109 prephitmp.12 ] [109])
	GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 2)
	reload_in_reg: (reg:SI 40 %f8 [orig:109 prephitmp.12 ] [109])
	reload_reg_rtx: (reg:SI 11 %o3)

Reloads for insn # 85
Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 30 %fp)
                                                    (const_int -6140 
[0xffffffffffffe804]))
	GENERAL_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 0), can't combine
	reload_in_reg: (plus:SI (reg/f:SI 30 %fp)
                                                    (const_int -6140 
[0xffffffffffffe804]))
	reload_reg_rtx: (reg:SI 11 %o3)
Reload 1: reload_in (SI) = (reg:SI 40 %f8 [orig:109 prephitmp.12 ] [109])
	GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 0)
	reload_in_reg: (reg:SI 40 %f8 [orig:109 prephitmp.12 ] [109])
	reload_reg_rtx: (reg:SI 11 %o3)

Reload 1 of insn #85 inherits the reload reg from reload 1 of insn #84, the
bug being that the same reload reg is also used for reload 0 of insn #85.

This is supposed to work like so: the inheritance code in choose_reload_regs
calls free_for_value_p with IGNORE_ADDRESS_RELOADS set to 1 to bypass the
conflict with reload 0 because there is special code in the same function to
discard reload 0 in this case:

	  /* If we can inherit a RELOAD_FOR_INPUT, or can use a
	     reload_override_in, then we do not need its related
	     RELOAD_FOR_INPUT_ADDRESS / RELOAD_FOR_INPADDR_ADDRESS reloads;
	     likewise for other reload types.
	     We handle this by removing a reload when its only replacement
	     is mentioned in reload_in of the reload we are going to inherit.
	     A special case are auto_inc expressions; even if the input is
	     inherited, we still need the address for the output.  We can
	     recognize them because they have RELOAD_OUT set to RELOAD_IN.
            If we succeeded removing some reload and we are doing a preliminary
	     pass just to remove such reloads, make another pass, since the
	     removal of one reload might allow us to inherit another one.  */
	  else if (rld[r].in
		   && rld[r].out != rld[r].in
		   && remove_address_replacements (rld[r].in) && pass)
	    pass = 2;

But it is called with rld[r].in equal to:
(gdb) p debug_rtx(in_rtx)
  (reg:SI 40 %f8 [orig:109 prephitmp.12 ] [109])

and the following replacement for reload 0:
(gdb) p debug_rtx(*replacements[0].where)
  (plus:SI (reg/f:SI 30 %fp)
        (const_int -6140 [0xffffffffffffe804]))

so loc_mentioned_in_p returns false and reload 0 isn't discarded.

The problem is that reload 0 is pushed because of SECONDARY_MEMORY_NEEDED,
i.e. (reg:SI 40 %f8) is spilled to memory at the address
(gdb) p debug_rtx(secondary_memlocs_elim[11][0])
  (mem/c:SI (plus:SI (reg/f:SI 30 %fp)
        (const_int -6140 [0xffffffffffffe804])) [0 S4 A32])

and replacements[0].where above points to within this MEM instead of IN_RTX.

The attached patch fixes the problem (on the 4.6 branch) by also invoking 
remove_address_replacements on the result of get_secondary_mem if a secondary 
memory is needed.  The condition to detect it has been copied from push_reload 
on the 4.6 branch, but it has already slightly changed on the mainline and I'm 
not sure how to adjust it.

Bootstrapped/regtested on x86_64-suse-linux.  Does that look plausible?  Do we 
want to fix this on release branches as well?


2012-09-02  Eric Botcazou  <ebotcazou@adacore.com>

	PR rtl-optimization/54290
	* reload1.c (choose_reload_regs): Also take into account secondary MEMs
	to remove address replacements for inherited reloads.


-- 
Eric Botcazou

Index: reload1.c
===================================================================
--- reload1.c	(revision 190664)
+++ reload1.c	(working copy)
@@ -6977,10 +6977,27 @@ choose_reload_regs (struct insn_chain *c
 	     If we succeeded removing some reload and we are doing a preliminary
 	     pass just to remove such reloads, make another pass, since the
 	     removal of one reload might allow us to inherit another one.  */
-	  else if (rld[r].in
+	  else if (pass
+		   && rld[r].in
 		   && rld[r].out != rld[r].in
-		   && remove_address_replacements (rld[r].in) && pass)
+		   && remove_address_replacements (rld[r].in))
 	    pass = 2;
+#ifdef SECONDARY_MEMORY_NEEDED
+	  else if (pass
+	           && rld[r].in
+		   && rld[r].out != rld[r].in
+		   && (REG_P (rld[r].in)
+		       || (GET_CODE (rld[r].in) == SUBREG
+			   && REG_P (SUBREG_REG (rld[r].in))))
+		   && reg_or_subregno (rld[r].in) < FIRST_PSEUDO_REGISTER
+		   && SECONDARY_MEMORY_NEEDED
+		      (REGNO_REG_CLASS (reg_or_subregno (rld[r].in)),
+				        rld[r].rclass, rld[r].inmode)
+		   && remove_address_replacements
+		      (get_secondary_mem (rld[r].in, rld[r].inmode,
+					  rld[r].opnum, rld[r].when_needed)))
+	    pass = 2;
+#endif
 	}
     }