diff mbox

[PR,target/48551] Make double_reg_address_ok a per-mode test

Message ID 9e6c6018-6c60-63b0-dfee-bbbeb3fcf14f@redhat.com
State New
Headers show

Commit Message

Jeff Law Nov. 21, 2016, 5:30 a.m. UTC 
Sigh.  Reload.

Reload has the assumption that if reg+reg addressing is valid in QImode 
that it is valid in all modes.  This has been baked into reload as long 
as I can remember.

It's part of a larger problem that reload doesn't handle some cases 
where address validity is dependent upon the context in which the 
address is used.  I'm not going to try to solve that problem as a whole, 
but I can make this tiny piece better.

In BZ48551 we have a case where we want to reload an address like
(plus (fp) (-largeint)) for a DFmode memory access.  We need the reload 
because the target has a limited range for displacements.

Reload has multiple strategies for handling this situation, including 
loading the integer into an index register and using reg+reg style 
addressing.  This case is predicated on checking double_reg_address_ok.

The problem is when we initialize double_reg_address_ok, we only do so 
for QImode.  So if the target only allows reg+reg style addressing in 
QImode, but not other modes modes, then we can end up generating bogus 
RTL leading to an insn recognition failure as seen in this BZ as well as 
building libgfortran on m68k-elf.

The easy solution here is to have double_reg_address_ok for all modes.

In this particular case that will cause reload to compute (plus (fp) 
-largeint) into an address register, then use simple register indirect.

This should be safe on other ports still using reload.  If they have 
properties similar to coldfire, then this could potentially avoid the 
same problem on those ports.  If they were the opposite (reg+reg not 
allowed for QImode, but is allowed in other modes), then this could 
allows those ports to generate slightly more efficient code.  Either way 
it seems like the right thing to do.

This fixes BZ 48551 as well as the libgfortran build failures for 
m68k-elf.  Another win for retro-computing.  Verified against the 
testcase in BZ48551 as well as building libgfortran for m68k-elf.

Then I went back to gcc-4.7, which is pre-lra and did a bootstrap and 
comparison test on x86_64-linux-gnu to exercise the code path a little 
harder.  That (of course) was successful.

Installing on the trunk.

Jeff
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 91cbe82..40d6f8a 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,13 @@ 
+2016-11-20  Jeff Law  <law@redhat.com>
+
+	PR target/48551
+	* reload.h (struct target_reload): Make x_double_reg_address_ok
+	be per-mode rather.
+	* reload.c (find_reloads_address): Check if double_reg_address_ok
+	is true for the mode of the memory reference.
+	* reload1.c (init_reload): Initialize double_reg_address_ok for
+	each mode.
+
 2016-11-20  Aldy Hernandez  <aldyh@redhat.com>
 
 	PR middle-end/61409
diff --git a/gcc/reload.c b/gcc/reload.c
index 7d16817..4cba220 100644
--- a/gcc/reload.c
+++ b/gcc/reload.c
@@ -5090,7 +5090,7 @@  find_reloads_address (machine_mode mode, rtx *memrefloc, rtx ad,
 	    loc = &XEXP (*loc, 0);
 	}
 
-      if (double_reg_address_ok
+      if (double_reg_address_ok[mode]
 	  && regno_ok_for_base_p (REGNO (XEXP (ad, 0)), mode, as,
 				  PLUS, CONST_INT))
 	{
diff --git a/gcc/reload.h b/gcc/reload.h
index 98b75e3..1fc8ecb 100644
--- a/gcc/reload.h
+++ b/gcc/reload.h
@@ -159,9 +159,6 @@  struct target_reload {
      which these are valid is the same as spill_indirect_levels, above.  */
   bool x_indirect_symref_ok;
 
-  /* Nonzero if an address (plus (reg frame_pointer) (reg ...)) is valid.  */
-  bool x_double_reg_address_ok;
-
   /* Nonzero if indirect addressing is supported on the machine; this means
      that spilling (REG n) does not require reloading it into a register in
      order to do (MEM (REG n)) or (MEM (PLUS (REG n) (CONST_INT c))).  The
@@ -181,6 +178,10 @@  struct target_reload {
 		     [FIRST_PSEUDO_REGISTER]
 		     [MAX_MOVE_MAX / MIN_UNITS_PER_WORD + 1]);
 
+  /* Nonzero if an address (plus (reg frame_pointer) (reg ...)) is valid
+     in the given mode.  */
+  bool x_double_reg_address_ok[MAX_MACHINE_MODE];
+
   /* We will only make a register eligible for caller-save if it can be
      saved in its widest mode with a simple SET insn as long as the memory
      address is valid.  We record the INSN_CODE is those insns here since
diff --git a/gcc/reload1.c b/gcc/reload1.c
index 4ee3840..b855268 100644
--- a/gcc/reload1.c
+++ b/gcc/reload1.c
@@ -448,11 +448,10 @@  init_reload (void)
       /* This way, we make sure that reg+reg is an offsettable address.  */
       tem = plus_constant (Pmode, tem, 4);
 
-      if (memory_address_p (QImode, tem))
-	{
-	  double_reg_address_ok = 1;
-	  break;
-	}
+      for (int mode = 0; mode < MAX_MACHINE_MODE; mode++)
+	if (!double_reg_address_ok[mode]
+	    && memory_address_p ((enum machine_mode)mode, tem))
+	  double_reg_address_ok[mode] = 1;
     }
 
   /* Initialize obstack for our rtl allocation.  */
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 6a23d90..37e8403 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2016-11-20  Jeff Law  <law@redhat.com>
+
+	PR target/48551
+	* gcc.target/m68k/pr48551.c: New test.
+
 2016-11-20  Harald Anlauf  <anlauf@gmx.de>
  
 	PR fortran/69741
diff --git a/gcc/testsuite/gcc.target/m68k/pr48551.c b/gcc/testsuite/gcc.target/m68k/pr48551.c
new file mode 100644
index 0000000..48ea4b4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/m68k/pr48551.c
@@ -0,0 +1,44 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -mcpu=5475" } */
+
+/* This tickles a problem with reload on the m68k.  There's a reasonable
+   chance it will get stale over time.  */
+
+int frob;
+typedef double SplashCoord;
+void transform (SplashCoord xi, SplashCoord yi);
+void
+arf (SplashCoord x0, SplashCoord y0, SplashCoord x1, SplashCoord y1,
+     SplashCoord x2, SplashCoord y2, SplashCoord x3, SplashCoord y3,
+     SplashCoord * matrix, SplashCoord flatness2)
+{
+  SplashCoord cx[(1 << 10) + 1][3];
+  SplashCoord cy[(1 << 10) + 1][3];
+  SplashCoord xl0, xl1, xl2, xr0, xr1, xr2, xr3, xx1, xx2, xh;
+  SplashCoord yl0, yl1, yl2, yr0, yr1, yr2, yr3, yy1, yy2, yh;
+  int p1, p2, p3;
+  while (p1  < (1 << 10))
+    {
+      xl0 = cx[p1][0];
+      xx2 = cx[p1][2];
+      yy2 = cy[p1][2];
+      transform (xx2, yy2);
+      if (frob)
+	{
+	  xl1 = (xl0 + xx1);
+	  xh = (xx1 + xx2);
+	  yl2 = (yl1 + yh);
+	  xr2 = (xx2 + xr3);
+	  yr2 = (yy2 + yr3) * 0.5;
+	  xr1 = (xh + xr2);
+	  yr1 = (yh + yr2);
+	  xr0 = (xl2 + xr1);
+	  yr0 = (yl2 + yr1);
+	  cx[p1][1] = xl1;
+	  cy[p1][1] = yl1;
+	  cx[p1][2] = xl2;
+	  cx[p3][0] = xr0;
+	  cy[p3][0] = yr0;
+	}
+    }
+}