diff mbox

[RELOAD] Don't assume subreg mem address is ok

Message ID 20160808024318.GX20904@bubble.grove.modra.org
State New
Headers show

Commit Message

Alan Modra Aug. 8, 2016, 2:43 a.m. UTC 
This patch fixes a case where reload blindly assumes a subreg mem
is OK if its address has been partially reloaded by
legitimize_reload_address.  The change ought to be fairly obvious
considering that the analogous non-subreg case only gives a win to
address_reloaded[i] == 1.  Bootstrapped and regression tested
powerpc64le-linux using
  RUNTESTFLAGS="--target_board=unix'{-mlra,-mno-lra}'"
to run the testsuite using both lra and reload.  OK for mainline
and active branches?

In the testcase we see this insn
(insn 316 272 349 38 (parallel [
            (set (reg:SF 336 [ _46 ])
                (unsigned_float:SF (subreg/s/v:HI (reg:DI 360 [ g ]) 0)))
            (clobber (scratch:DI))
            (clobber (scratch:DI))
            (clobber (scratch:HI))
        ]) 358 {*floatunshisf2_internal}
not getting a hard reg for reg 360, which has an equivalent constant.
Reload substitutes the constant, but the pattern does not allow
constants so it is forced to mem.  The mem address is legitimized by
rs6000_legitimize_reload_address to a lo_sum/high constant pool ref.
But this particular address form is not valid for the pattern, which
wants either an indirect or indexed address.

	PR rtl-optimization/72771
	* reload.c (find_reloads): Don't assume that a subreg mem is OK
	when find_reloads_toplev returns address_reloaded==-1.
	(alternative_allows_const_pool_ref): Update comment.
testsuite/
	* gcc.c-torture/compile/pr72771.c: New.

Comments

Alan Modra Aug. 8, 2016, 3:25 a.m. UTC | #1 
On Mon, Aug 08, 2016 at 12:13:18PM +0930, Alan Modra wrote:
> This patch fixes a case where reload blindly assumes a subreg mem
> is OK if its address has been partially reloaded by
> legitimize_reload_address.  The change ought to be fairly obvious
> considering that the analogous non-subreg case only gives a win to
> address_reloaded[i] == 1.  Bootstrapped and regression tested
> powerpc64le-linux using
>   RUNTESTFLAGS="--target_board=unix'{-mlra,-mno-lra}'"
> to run the testsuite using both lra and reload.  OK for mainline
> and active branches?

When I look at the correct bootstrap I see I have some regressions.
Sorry for the noise, patch withdrawn.
diff mbox

Patch

diff --git a/gcc/reload.c b/gcc/reload.c
index 1945133..bcd15a1 100644
--- a/gcc/reload.c
+++ b/gcc/reload.c
@@ -3960,7 +3960,7 @@  find_reloads (rtx_insn *insn, int replace, int ind_levels, int live_known,
 	       there will be no reload needed at all.  */
 	    if (plus == NULL_RTX
 		&& subreg == NULL_RTX
-		&& alternative_allows_const_pool_ref (this_address_reloaded == 0
+		&& alternative_allows_const_pool_ref (this_address_reloaded != 1
 						      ? substed_operand[i]
 						      : NULL,
 						      recog_data.constraints[i],
@@ -4605,8 +4605,8 @@  find_reloads (rtx_insn *insn, int replace, int ind_levels, int live_known,
 
 /* Return true if alternative number ALTNUM in constraint-string
    CONSTRAINT is guaranteed to accept a reloaded constant-pool reference.
-   MEM gives the reference if it didn't need any reloads, otherwise it
-   is null.  */
+   MEM gives the reference if its address hasn't been fully reloaded,
+   otherwise it is NULL.  */
 
 static bool
 alternative_allows_const_pool_ref (rtx mem ATTRIBUTE_UNUSED,
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr72771.c b/gcc/testsuite/gcc.c-torture/compile/pr72771.c
new file mode 100644
index 0000000..1cc13e3
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr72771.c
@@ -0,0 +1,29 @@ 
+void fn2(void);
+void fn3(unsigned long);
+
+signed char a;
+int b, c, e, f;
+float *d;
+
+void fn1(void) {
+        unsigned short g = 0;
+lbl_986:
+        for (;;) {
+                c = 0;
+                for (; c <= 2;) {
+                        *d = g;
+                        fn2();
+                        if (e)
+                                goto lbl_986;
+                }
+                g = 2;
+                for (; (short) g >= 0; g--) {
+                        for (; b;) {
+                                fn3(45360);
+                                f = 0;
+                                for (; a >= 0; a--)
+                                        ;
+                        }
+                }
+        }
+}