[01/31] PR target/58901: reload: Handle SUBREG of MEM with a mode-dependent address

Message ID alpine.LFD.2.21.2011200228570.656242@eddie.linux-mips.org
State New
Headers show
Series
  • VAX: Bring the port up to date (yes, MODE_CC conversion is included)
Related show

Commit Message

Maciej W. Rozycki Nov. 20, 2020, 3:34 a.m.
From: Matt Thomas <matt@3am-software.com>


Fix an ICE with the handling of RTL expressions like:

(subreg:QI (mem/c:SI (plus:SI (plus:SI (mult:SI (reg/v:SI 0 %r0 [orig:67 i ] [67])
                    (const_int 4 [0x4]))
                (reg/v/f:SI 7 %r7 [orig:59 doacross ] [59]))
            (const_int 40 [0x28])) [1 MEM[(unsigned int *)doacross_63 + 40B + i_106 * 4]+0 S4 A32]) 0)

that causes the compilation of libgomp to fail:

during RTL pass: reload
.../libgomp/ordered.c: In function 'GOMP_doacross_wait':
.../libgomp/ordered.c:507:1: internal compiler error: in change_address_1, at emit-rtl.c:2275
  507 | }
      | ^
0x10a3462b change_address_1
	.../gcc/emit-rtl.c:2275
0x10a353a7 adjust_address_1(rtx_def*, machine_mode, poly_int<1u, long>, int, int, int, poly_int<1u, long>)
	.../gcc/emit-rtl.c:2409
0x10ae2993 alter_subreg(rtx_def**, bool)
	.../gcc/final.c:3368
0x10ae25cf cleanup_subreg_operands(rtx_insn*)
	.../gcc/final.c:3322
0x110922a3 reload(rtx_insn*, int)
	.../gcc/reload1.c:1232
0x10de2bf7 do_reload
	.../gcc/ira.c:5812
0x10de3377 execute
	.../gcc/ira.c:5986

in a `vax-netbsdelf' build, where an attempt is made to change the mode
of the contained memory reference to the mode of the containing SUBREG.
Such RTL expressions are produced by the VAX shift and rotate patterns
(`ashift', `ashiftrt', `rotate', `rotatert') where the count operand
always has the QI mode regardless of the mode, either SI or DI, of the
datum shifted or rotated.

Such a mode change cannot work where the memory reference uses the
indexed addressing mode, where a multiplier is implied that in the VAX
ISA depends on the width of the memory access requested and therefore
changing the machine mode would change the address calculation as well.

Avoid the attempt then by forcing the reload of any SUBREGs containing
a mode-dependent memory reference, also fixing these regressions:

FAIL: gcc.c-torture/compile/pr46883.c   -Os  (internal compiler error)
FAIL: gcc.c-torture/compile/pr46883.c   -Os  (test for excess errors)
FAIL: gcc.c-torture/execute/20120808-1.c   -O2  (internal compiler error)
FAIL: gcc.c-torture/execute/20120808-1.c   -O2  (test for excess errors)
FAIL: gcc.c-torture/execute/20120808-1.c   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  (internal compiler error)
FAIL: gcc.c-torture/execute/20120808-1.c   -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions  (test for excess errors)
FAIL: gcc.c-torture/execute/20120808-1.c   -O3 -g  (internal compiler error)
FAIL: gcc.c-torture/execute/20120808-1.c   -O3 -g  (test for excess errors)
FAIL: gcc.c-torture/execute/20120808-1.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  (internal compiler error)
FAIL: gcc.c-torture/execute/20120808-1.c   -O2 -flto -fno-use-linker-plugin -flto-partition=none  (test for excess errors)
FAIL: gcc.c-torture/execute/20120808-1.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  (internal compiler error)
FAIL: gcc.c-torture/execute/20120808-1.c   -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects  (test for excess errors)
FAIL: gcc.dg/20050629-1.c (internal compiler error)
FAIL: gcc.dg/20050629-1.c (test for excess errors)
FAIL: c-c++-common/torture/pr53505.c   -Os  (internal compiler error)
FAIL: c-c++-common/torture/pr53505.c   -Os  (test for excess errors)
FAIL: gfortran.dg/coarray_failed_images_1.f08   -Os  (internal compiler error)
FAIL: gfortran.dg/coarray_stopped_images_1.f08   -Os  (internal compiler error)

First posted at: <https://gcc.gnu.org/ml/gcc/2014-06/msg00060.html>.

	gcc/
	PR target/58901
	* reload.c (reload_inner_reg_of_subreg): Also request reloading
	for pseudo registers associated with mode dependent memory
	references.
	(push_reload): Handle pseudo registers.
---
 gcc/reload.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

-- 
2.11.0

Comments

Eric Botcazou Nov. 20, 2020, 10:55 a.m. | #1
> First posted at: <https://gcc.gnu.org/ml/gcc/2014-06/msg00060.html>.


> 	gcc/

> 	PR target/58901

> 	* reload.c (reload_inner_reg_of_subreg): Also request reloading

> 	for pseudo registers associated with mode dependent memory

> 	references.

> 	(push_reload): Handle pseudo registers.


The handling of this family of reloads is supposed to be done by the block of 
code just above though, i.e. at line 1023.  Can't we add the test based on 
mode_dependent_address_p to this block, e.g. after:

	  || (REG_P (SUBREG_REG (in))
	      && REGNO (SUBREG_REG (in)) < FIRST_PSEUDO_REGISTER
	      && !REG_CAN_CHANGE_MODE_P (REGNO (SUBREG_REG (in)),
					 GET_MODE (SUBREG_REG 
(in)), inmode))))

instead?

-- 
Eric Botcazou
Maciej W. Rozycki Nov. 20, 2020, 3:30 p.m. | #2
On Fri, 20 Nov 2020, Eric Botcazou wrote:

> > 	gcc/

> > 	PR target/58901

> > 	* reload.c (reload_inner_reg_of_subreg): Also request reloading

> > 	for pseudo registers associated with mode dependent memory

> > 	references.

> > 	(push_reload): Handle pseudo registers.

> 

> The handling of this family of reloads is supposed to be done by the block of 

> code just above though, i.e. at line 1023.  Can't we add the test based on 

> mode_dependent_address_p to this block, e.g. after:

> 

> 	  || (REG_P (SUBREG_REG (in))

> 	      && REGNO (SUBREG_REG (in)) < FIRST_PSEUDO_REGISTER

> 	      && !REG_CAN_CHANGE_MODE_P (REGNO (SUBREG_REG (in)),

> 					 GET_MODE (SUBREG_REG 

> (in)), inmode))))

> 

> instead?


 Thank you for your input, I'll have a look.  Coming from Matt this is the 
only change of the series I have just merged without looking into it too 
much, so as not to spend too much time with side issues (there were too 
many already).

 It'll take me a couple of days to push the new version through regression 
testing, and I'll post it once that is complete along with any other 
updates someone may request.

  Maciej

Patch

diff --git a/gcc/reload.c b/gcc/reload.c
index 445f9bdca43..dbf83733815 100644
--- a/gcc/reload.c
+++ b/gcc/reload.c
@@ -838,6 +838,7 @@  static bool
 reload_inner_reg_of_subreg (rtx x, machine_mode mode, bool output)
 {
   rtx inner;
+  int regno;
 
   /* Only SUBREGs are problematical.  */
   if (GET_CODE (x) != SUBREG)
@@ -849,10 +850,21 @@  reload_inner_reg_of_subreg (rtx x, machine_mode mode, bool output)
   if (CONSTANT_P (inner) || GET_CODE (inner) == PLUS)
     return true;
 
-  /* If INNER is not a hard register, then INNER will not need reloading.  */
-  if (!(REG_P (inner) && HARD_REGISTER_P (inner)))
+  /* If INNER is not a register, then INNER will not need reloading.  */
+  if (!REG_P (inner))
     return false;
 
+  regno = REGNO (inner);
+
+  /* If INNER is not a hard register, then INNER will not need reloading
+     unless it's a mode dependent memory reference.  */
+  if (regno >= FIRST_PSEUDO_REGISTER)
+    return (!output
+	    && reg_equiv_mem (regno) != 0
+	    && (mode_dependent_address_p
+		(XEXP (reg_equiv_mem (regno), 0),
+		 MEM_ADDR_SPACE (reg_equiv_mem (regno)))));
+
   /* If INNER is not ok for MODE, then INNER will need reloading.  */
   if (!targetm.hard_regno_mode_ok (subreg_regno (x), mode))
     return true;
@@ -1119,7 +1131,7 @@  push_reload (rtx in, rtx out, rtx *inloc, rtx *outloc,
 
   if (in != 0 && reload_inner_reg_of_subreg (in, inmode, false))
     {
-      if (REG_P (SUBREG_REG (in)))
+      if (REG_P (SUBREG_REG (in)) && HARD_REGISTER_P (SUBREG_REG (in)))
 	subreg_in_class
 	  = find_valid_class (inmode, GET_MODE (SUBREG_REG (in)),
 			      subreg_regno_offset (REGNO (SUBREG_REG (in)),
@@ -1127,8 +1139,9 @@  push_reload (rtx in, rtx out, rtx *inloc, rtx *outloc,
 						   SUBREG_BYTE (in),
 						   GET_MODE (in)),
 			      REGNO (SUBREG_REG (in)));
-      else if (CONSTANT_P (SUBREG_REG (in))
-               || GET_CODE (SUBREG_REG (in)) == PLUS)
+      else if (REG_P (SUBREG_REG (in))
+	       || CONSTANT_P (SUBREG_REG (in))
+	       || GET_CODE (SUBREG_REG (in)) == PLUS)
 	subreg_in_class = find_valid_class_1 (inmode,
 					      GET_MODE (SUBREG_REG (in)),
 					      rclass);