diff mbox

[ARM] Fix PR48808, PR48792: More work on CANNOT_CHANGE_MODE_CLASS

Message ID 87ei3hjf52.fsf@firetop.home
State New
Headers show

Commit Message

Richard Sandiford May 29, 2011, 10:50 p.m. UTC 
Eric Botcazou <ebotcazou@adacore.com> writes:
>> Something like the attached patch.  Not tested yet, and I'm sure
>> it'll break things in lots of fun and interesting ways...
>
> Mind posting a (temporarily) definitive version?  I'll give it a whirl on the 
> SPARC and IA-64 to see how it would fare.

Thanks.  With the s/MODE/CODE/ fix (attached) it passes bootstrap &
regression test on x86_64-linux-gnu.  I'll also try to some assembly

Comments

Richard Sandiford May 30, 2011, 4:35 p.m. UTC | #1 
Richard Sandiford <rdsandiford@googlemail.com> writes:
> Eric Botcazou <ebotcazou@adacore.com> writes:
>>> Something like the attached patch.  Not tested yet, and I'm sure
>>> it'll break things in lots of fun and interesting ways...
>>
>> Mind posting a (temporarily) definitive version?  I'll give it a whirl on the 
>> SPARC and IA-64 to see how it would fare.
>
> Thanks.  With the s/MODE/CODE/ fix (attached) it passes bootstrap &
> regression test on x86_64-linux-gnu.  I'll also try to some assembly
> diffs over a range of targets.

FWIW, I tried compiling gcc.c-torture, gcc.dg and g++.dg at -O2
for these targets:

	alpha-linux-gnu arm-linux-gnueabi avr-rtems bfin-elf
	cris-elf fr30-elf frv-linux-gnu h8300-elf ia64-linux-gnu
	iq2000-elf lm32-elf m32c-elf m32r-elf m68k-linux-gnu
	mcore-elf mep-elf microblaze-elf mips-linux-gnu mmix
	mn10300-elf moxie-elf hppa64-hp-hpux11.23 pdp11 picochip-elf
	powerpc-linux-gnu powerpc-eabispe rx-elf s390-linux-gnu
	score-elf sh-linux-gnu sparc-linux-gnu spu-elf xstormy16-elf
	v850-elf vax-netbsdelf xtensa-elf

It's a bit of a flawed exercise, because I don't have appropriate
system headers for most of them.  But of the tests that did compile,
there were no differences in assembly output and no new ICEs.

Richard
Eric Botcazou May 30, 2011, 6:36 p.m. UTC | #2 
> 	* reload.c (push_reload): Check contains_reg_of_mode.
> 	* reload1.c (strip_paradoxical_subreg): New function.
> 	(gen_reload_chain_without_interm_reg_p): Use it to handle
> 	paradoxical subregs.
> 	(emit_output_reload_insns, gen_reload): Likewise.

Testing (not a full cycle, but still) revealed no problems on SPARC (32-bit and 
64-bit) or IA-64.  The patch is OK as far as I'm concerned, but you might want 
to get a second opinion from a reload expert.

Now I have a couple of requests:

  1. Could you add PR rtl-optimization/48830 to the ChangeLog and install the 
testcase (attached) distilled by Hans-Peter as gcc.target/sparc/ultrasp12.c?

  2. Could you rename the first parameter of the new function?  "focus" sounds 
a little strange to me and is unheard of in the GCC codebase.  Maybe "target"?

Thanks for working on this.
diff mbox

Patch

diffs over a range of targets.

Richard


gcc/
	* reload.c (push_reload): Check contains_reg_of_mode.
	* reload1.c (strip_paradoxical_subreg): New function.
	(gen_reload_chain_without_interm_reg_p): Use it to handle
	paradoxical subregs.
	(emit_output_reload_insns, gen_reload): Likewise.

Index: gcc/reload.c
===================================================================
--- gcc/reload.c	2011-05-29 23:48:02.000000000 +0100
+++ gcc/reload.c	2011-05-29 23:48:02.000000000 +0100
@@ -1019,6 +1019,7 @@  push_reload (rtx in, rtx out, rtx *inloc
 #ifdef CANNOT_CHANGE_MODE_CLASS
       && !CANNOT_CHANGE_MODE_CLASS (GET_MODE (SUBREG_REG (in)), inmode, rclass)
 #endif
+      && contains_reg_of_mode[(int) rclass][(int) GET_MODE (SUBREG_REG (in))]
       && (CONSTANT_P (SUBREG_REG (in))
 	  || GET_CODE (SUBREG_REG (in)) == PLUS
 	  || strict_low
@@ -1125,6 +1126,7 @@  push_reload (rtx in, rtx out, rtx *inloc
 #ifdef CANNOT_CHANGE_MODE_CLASS
       && !CANNOT_CHANGE_MODE_CLASS (GET_MODE (SUBREG_REG (out)), outmode, rclass)
 #endif
+      && contains_reg_of_mode[(int) rclass][(int) GET_MODE (SUBREG_REG (out))]
       && (CONSTANT_P (SUBREG_REG (out))
 	  || strict_low
 	  || (((REG_P (SUBREG_REG (out))
Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	2011-05-29 23:48:02.000000000 +0100
+++ gcc/reload1.c	2011-05-29 23:48:02.000000000 +0100
@@ -4471,6 +4471,43 @@  scan_paradoxical_subregs (rtx x)
 	}
     }
 }
+
+/* *FOCUS_PTR and *OTHER_PTR are two operands to a conceptual reload.
+   If *FOCUS_PTR is a paradoxical subreg, try to remove that subreg
+   and apply the corresponding narrowing subreg to *OTHER_PTR.
+   Return true if the operands were changed, false otherwise.  */
+
+static bool
+strip_paradoxical_subreg (rtx *focus_ptr, rtx *other_ptr)
+{
+  rtx focus, inner, other, tem;
+
+  focus = *focus_ptr;
+  if (GET_CODE (focus) != SUBREG)
+    return false;
+
+  inner = SUBREG_REG (focus);
+  if (GET_MODE_SIZE (GET_MODE (focus)) <= GET_MODE_SIZE (GET_MODE (inner)))
+    return false;
+
+  other = *other_ptr;
+  tem = gen_lowpart_common (GET_MODE (inner), other);
+  if (!tem)
+    return false;
+
+  /* If the lowpart operation turned a hard register into a subreg,
+     rather than simplifying it to another hard register, then the
+     mode change cannot be properly represented.  For example, OTHER
+     might be valid in its current mode, but not in the new one.  */
+  if (GET_CODE (tem) == SUBREG
+      && REG_P (other)
+      && HARD_REGISTER_P (other))
+    return false;
+
+  *focus_ptr = inner;
+  *other_ptr = tem;
+  return true;
+}
 
 /* A subroutine of reload_as_needed.  If INSN has a REG_EH_REGION note,
    examine all of the reload insns between PREV and NEXT exclusive, and
@@ -5538,7 +5575,7 @@  gen_reload_chain_without_interm_reg_p (i
      chain reloads or do need an intermediate hard registers.  */
   bool result = true;
   int regno, n, code;
-  rtx out, in, tem, insn;
+  rtx out, in, insn;
   rtx last = get_last_insn ();
 
   /* Make r2 a component of r1.  */
@@ -5557,11 +5594,7 @@  gen_reload_chain_without_interm_reg_p (i
 
   /* If IN is a paradoxical SUBREG, remove it and try to put the
      opposite SUBREG on OUT.  Likewise for a paradoxical SUBREG on OUT.  */
-  if (GET_CODE (in) == SUBREG
-      && (GET_MODE_SIZE (GET_MODE (in))
-	  > GET_MODE_SIZE (GET_MODE (SUBREG_REG (in))))
-      && (tem = gen_lowpart_common (GET_MODE (SUBREG_REG (in)), out)) != 0)
-    in = SUBREG_REG (in), out = tem;
+  strip_paradoxical_subreg (&in, &out);
 
   if (GET_CODE (in) == PLUS
       && (REG_P (XEXP (in, 0))
@@ -7557,7 +7590,6 @@  emit_output_reload_insns (struct insn_ch
 	      if (tertiary_icode != CODE_FOR_nothing)
 		{
 		  rtx third_reloadreg = rld[tertiary_reload].reg_rtx;
-		  rtx tem;
 
 		  /* Copy primary reload reg to secondary reload reg.
 		     (Note that these have been swapped above, then
@@ -7566,13 +7598,7 @@  emit_output_reload_insns (struct insn_ch
 		  /* If REAL_OLD is a paradoxical SUBREG, remove it
 		     and try to put the opposite SUBREG on
 		     RELOADREG.  */
-		  if (GET_CODE (real_old) == SUBREG
-		      && (GET_MODE_SIZE (GET_MODE (real_old))
-			  > GET_MODE_SIZE (GET_MODE (SUBREG_REG (real_old))))
-		      && 0 != (tem = gen_lowpart_common
-			       (GET_MODE (SUBREG_REG (real_old)),
-				reloadreg)))
-		    real_old = SUBREG_REG (real_old), reloadreg = tem;
+		  strip_paradoxical_subreg (&real_old, &reloadreg);
 
 		  gen_reload (reloadreg, second_reloadreg,
 			      rl->opnum, rl->when_needed);
@@ -8388,16 +8414,8 @@  gen_reload (rtx out, rtx in, int opnum,
 
   /* If IN is a paradoxical SUBREG, remove it and try to put the
      opposite SUBREG on OUT.  Likewise for a paradoxical SUBREG on OUT.  */
-  if (GET_CODE (in) == SUBREG
-      && (GET_MODE_SIZE (GET_MODE (in))
-	  > GET_MODE_SIZE (GET_MODE (SUBREG_REG (in))))
-      && (tem = gen_lowpart_common (GET_MODE (SUBREG_REG (in)), out)) != 0)
-    in = SUBREG_REG (in), out = tem;
-  else if (GET_CODE (out) == SUBREG
-	   && (GET_MODE_SIZE (GET_MODE (out))
-	       > GET_MODE_SIZE (GET_MODE (SUBREG_REG (out))))
-	   && (tem = gen_lowpart_common (GET_MODE (SUBREG_REG (out)), in)) != 0)
-    out = SUBREG_REG (out), in = tem;
+  if (!strip_paradoxical_subreg (&in, &out))
+    strip_paradoxical_subreg (&out, &in);
 
   /* How to do this reload can get quite tricky.  Normally, we are being
      asked to reload a simple operand, such as a MEM, a constant, or a pseudo