Bug 54699 - [12/13/14/15 Regression] [SH] gfortran.dg/class_array_9.f03 ICEs
Summary: [12/13/14/15 Regression] [SH] gfortran.dg/class_array_9.f03 ICEs
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.8.0
: P4 normal
Target Milestone: 12.5
Assignee: Oleg Endo
URL:
Keywords: ice-on-valid-code
Depends on: 55212
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-25 08:43 UTC by Kazumoto Kojima
Modified: 2024-07-19 12:55 UTC (History)
0 users

See Also:
Host:
Target: sh*-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-10-09 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kazumoto Kojima 2012-09-25 08:43:13 UTC
gfortran.dg/class_array_9.f03 ICEs for a while on sh4-unknown-linux-gnu.

FAIL: gfortran.dg/class_array_9.f03  -O0  (internal compiler error)

gfortran.log says:

internal compiler error: in change_address_1, at emit-rtl.c:2006

Here is a gdb's backtrace for f951:

(gdb) bt
#0  internal_error (gmsgid=0x8abd243 "in %s, at %s:%d")
    at ../../ORIG/trunk/gcc/diagnostic.c:951
#1  0x0897228f in fancy_abort (
    file=0x89fedb8 "../../ORIG/trunk/gcc/emit-rtl.c", line=2006, 
    function=0x89feba1 "change_address_1")
    at ../../ORIG/trunk/gcc/diagnostic.c:1011
#2  0x0832006b in change_address_1 (memref=0xb7db49f0, mode=SFmode, 
    addr=0xb7dcfbe8, validate=1) at ../../ORIG/trunk/gcc/emit-rtl.c:2006
#3  0x08323c9e in adjust_address_1 (memref=0xb7db49f0, mode=SFmode, offset=4, 
    validate=1, adjust_address=1, adjust_object=0)
    at ../../ORIG/trunk/gcc/emit-rtl.c:2111
#4  0x0836ea01 in alter_subreg (xp=0xb7dcf850)
    at ../../ORIG/trunk/gcc/final.c:3004
#5  0x0836ec50 in cleanup_subreg_operands (insn=0xb7dd0384)
    at ../../ORIG/trunk/gcc/final.c:2951
#6  0x0852f941 in reload (first=0xb7dad680, global=0)
    at ../../ORIG/trunk/gcc/reload1.c:1241
#7  0x08468e49 in do_reload () at ../../ORIG/trunk/gcc/ira.c:4328
#8  rest_of_handle_reload () at ../../ORIG/trunk/gcc/ira.c:4419
#9  0x084e2305 in execute_one_pass (pass=0x8ba03e0)
    at ../../ORIG/trunk/gcc/passes.c:2199
#10 0x084e26a5 in execute_pass_list (pass=0x8ba03e0)
    at ../../ORIG/trunk/gcc/passes.c:2254
---Type <return> to continue, or q <return> to quit---q
Quit
(gdb) fr 2
#2  0x0832006b in change_address_1 (memref=0xb7db49f0, mode=SFmode, 
    addr=0xb7dcfbe8, validate=1) at ../../ORIG/trunk/gcc/emit-rtl.c:2006
2006		gcc_assert (memory_address_addr_space_p (mode, addr, as));
(gdb) p addr
$1 = (rtx_def *) 0xb7dcfbe8
(gdb) call debug_rtx(addr)
(plus:SI (reg/f:SI 14 r14)
    (const_int 4 [0x4]))

(gdb) fr 4
#4  0x0836ea01 in alter_subreg (xp=0xb7dcf850)
    at ../../ORIG/trunk/gcc/final.c:3004
3004	      *xp = adjust_address (y, GET_MODE (x), offset);
(gdb) p xp
$1 = (rtx *) 0xb7dcf850
(gdb) call debug_rtx(*xp)
(subreg:SF (mem/c:DI (reg/f:SI 14 r14) [0 %sfp+-200 S8 A32]) 4)

(gdb) fr 5
#5  0x0836ec50 in cleanup_subreg_operands (insn=0xb7dd0384)
    at ../../ORIG/trunk/gcc/final.c:2951
2951		  recog_data.operand[i] = alter_subreg (recog_data.operand_loc[i]);
(gdb) call debug_rtx(insn)
(insn 1124 661 1125 67 (parallel [
            (set (subreg:SF (mem/c:DI (reg/f:SI 14 r14) [0 %sfp+-200 S8 A32]) 4)
                (reg:SF 0 r0))
            (use (reg/v:PSI 151 ))
            (clobber (scratch:SI))
        ]) ch9.f03:38 269 {movsf_ie}
     (nil))

Looks that something wrong happens during reload and it requests
an illegal memory address r14+4 for SFmode on SH.
It started to fail between trunk revision 185498 and 185552.
Perhaps related to the changes for PR target/50751.
Comment 1 Oleg Endo 2012-09-25 11:56:53 UTC
(In reply to comment #0)

Thanks for tracing this.
The mem store insn

(set (subreg:SF (mem/c:DI (reg/f:SI 14 r14)) 4)
     (reg:SF 0 r0))

looks valid.  The "movsf_ie" pattern has alternatives for this combination.
However, the address in SFmode will not make it through sh_legitimate_address_p.

Legitimating the tuple (address, mode) without knowing the other operand and
whether it's a load or store is frustrating on SH.
I will try to see what's happening there.
Comment 2 Oleg Endo 2012-09-26 20:05:47 UTC
Hm, maybe implementing TARGET_MODE_DEPENDENT_ADDRESS_P would help this case?
Comment 3 Oleg Endo 2012-09-30 21:09:29 UTC
Doing this...

Index: gcc/config/sh/sh.c
===================================================================
--- gcc/config/sh/sh.c	(revision 191865)
+++ gcc/config/sh/sh.c	(working copy)
@@ -10079,6 +10079,9 @@
 static bool
 sh_legitimate_address_p (enum machine_mode mode, rtx x, bool strict)
 {
+  if (reload_completed)
+    return true;
+
   if (MAYBE_BASE_REGISTER_RTX_P (x, strict))
     return true;
   else if ((GET_CODE (x) == POST_INC || GET_CODE (x) == PRE_DEC)


makes the ICE go away.  However, I have not tested this for any other consequences.

This one is probably the better fix (max_mov_insn_displacement):

Index: gcc/config/sh/sh.c
===================================================================
--- gcc/config/sh/sh.c	(revision 191865)
+++ gcc/config/sh/sh.c	(working copy)
@@ -3457,21 +3457,20 @@
 
   /* SH2A supports FPU move insns with 12 bit displacements.
      Other variants to do not support any kind of displacements for
-     FPU move insns.  */
-  if (! consider_sh2a && TARGET_FPU_ANY && GET_MODE_CLASS (mode) == MODE_FLOAT)
-    return 0;
-  else
-    {
-      const int mov_insn_sz = mov_insn_size (mode, consider_sh2a);
-      const int mode_sz = GET_MODE_SIZE (mode);
-      int r = 15 * mov_insn_sz * disp_scale;
+     FPU move insns.  However, in the worst case, we can still use SImode
+     loads/stores with displacement, such as in the movsf_ie pattern instead
+     of true FPU move insns.  It's just going to be more expensive because
+     additional gp reg <-> fpu reg moves have to be used for that.  */
+  const int mov_insn_sz = mov_insn_size (mode, consider_sh2a);
+  const int mode_sz = GET_MODE_SIZE (mode);
+  int r = 15 * mov_insn_sz * disp_scale;
     
-      /* If the mov insn will be split into multiple loads/stores, the
-	 maximum possible displacement is a bit smaller.  */
-      if (mode_sz > mov_insn_sz)
-	r -= mode_sz - mov_insn_sz;
-      return r;
-    }
+  /* If the mov insn will be split into multiple loads/stores, the
+     maximum possible displacement is a bit smaller.  */
+  if (mode_sz > mov_insn_sz)
+    r -= mode_sz - mov_insn_sz;
+
+  return r;
 }
 
 /* Determine the alignment mask for a move insn of the


This makes the ICE go away, but it will wrongly output SH2A fmov.s insns such as
    fmov.s	@(16,r7),fr3

when compiling for SH4.

The movsf_ie insn looks a bit ... complicated ... and probably should be split into multiple insns to be able to handle such cases.

Maybe there's an even easier solution, but I can't imagine one at the moment.
Comment 4 Oleg Endo 2012-10-09 10:41:23 UTC
(In reply to comment #3)
> Doing this...
> 
> Index: gcc/config/sh/sh.c
> ===================================================================
> --- gcc/config/sh/sh.c    (revision 191865)
> +++ gcc/config/sh/sh.c    (working copy)
> @@ -10079,6 +10079,9 @@
>  static bool
>  sh_legitimate_address_p (enum machine_mode mode, rtx x, bool strict)
>  {
> +  if (reload_completed)
> +    return true;
> +
>    if (MAYBE_BASE_REGISTER_RTX_P (x, strict))
>      return true;
>    else if ((GET_CODE (x) == POST_INC || GET_CODE (x) == PRE_DEC)
> 
> 
> makes the ICE go away.  However, I have not tested this for any other
> consequences.
> 

I'm wondering whether there is anything after reload that actually needs address validation.  I guess that after the reload pass pretty much everything should have been fixed up which could generate invalid addresses that need to be transformed.  Of course, in one of the split passes after reload or the peephole2 pass somebody could write a pattern that would result in an invalid address.  But even now with the address checking after reload, if there is an invalid address left after reload, which pass would legitimize it?
Comment 5 Kazumoto Kojima 2012-10-10 00:03:54 UTC
(In reply to comment #4)
> I'm wondering whether there is anything after reload that actually needs
> address validation.  I guess that after the reload pass pretty much everything
> should have been fixed up which could generate invalid addresses that need to
> be transformed.  Of course, in one of the split passes after reload or the
> peephole2 pass somebody could write a pattern that would result in an invalid
> address.  But even now with the address checking after reload, if there is an
> invalid address left after reload, which pass would legitimize it?

*legitimate_address_p is a query function and possibly used to
verify some transformation of addressing.  It seems to me that

>  sh_legitimate_address_p (enum machine_mode mode, rtx x, bool strict)
>  {
> +  if (reload_completed)
> +    return true;

will open a can of worms.  With this change, I've got

../../../LOCAL/trunk/libgcc/libgcc2.c: In function '__muldc3':
../../../LOCAL/trunk/libgcc/libgcc2.c:1929:1: internal compiler error: Segmentation fault
 }
 ^
0x852c3a0 crash_signal
	../../LOCAL/trunk/gcc/toplev.c:335
0x8772b21 sh_print_operand_address
	../../LOCAL/trunk/gcc/config/sh/sh.c:1050
0x8308eba output_address(rtx_def*)
	../../LOCAL/trunk/gcc/final.c:3680

during building libgcc on sh4-unknown-linux-gnu.  gdb backtrace says

(gdb) fr 5
#5  0x083093a5 in output_asm_insn (templ=0x8a2621c "fmov.s\t%1,%0", 
    operands=0x8bc10c0) at ../../LOCAL/trunk/gcc/final.c:3562
3562		      output_operand (operands[opnum], 0);
(gdb) call debug_rtx(operands[0])
(mem/c:SF (plus:SI (plus:SI (reg:SI 0 r0)
            (reg/f:SI 15 r15))
        (const_int 4 [0x4])) [2 %sfp+-24 S8 A32])
(gdb) call debug_rtx(operands[1])

It looks we've got reg+reg+const addressing.  It seems that
reload_completed simply means that hard register are allocated
already but doesn't mean transformations of addressing are done.
Splitting movsf_ie would be the way to go, I guess.
Comment 6 Oleg Endo 2012-10-10 00:22:54 UTC
(In reply to comment #5)
> 
> It looks we've got reg+reg+const addressing.  It seems that
> reload_completed simply means that hard register are allocated
> already but doesn't mean transformations of addressing are done.
> Splitting movsf_ie would be the way to go, I guess.

Thanks for trying it out.  Good to know.
It's just that the comment above movsf_ie...

;; We may not split the ry/yr/XX alternatives to movsi_ie, since
;; update_flow_info would not know where to put REG_EQUAL notes
;; when the destination changes mode.

sounds a bit scary.  However...

grep update_flow_info gcc/*

gcc/ChangeLog-1999:	(update_flow_info): Move to flow.c, renamed to update_life_info;

That was quite a while ago... maybe it isn't a problem anymore.
Comment 7 Oleg Endo 2013-01-29 21:12:56 UTC
I've checked this again on rev. 195555 and the problem doesn't seem to happen anymore.  It also doesn't show up in the latest SH4 test result post

http://gcc.gnu.org/ml/gcc-testresults/2013-01/msg03050.html

That doesn't fix the real problem of course, and it might popup again.
I'll try to fix it on an older revision where the problem shows up and then apply it to trunk.
Comment 8 Jakub Jelinek 2013-03-22 14:46:05 UTC
GCC 4.8.0 is being released, adjusting target milestone.
Comment 9 Jakub Jelinek 2013-05-31 10:58:59 UTC
GCC 4.8.1 has been released.
Comment 10 Jakub Jelinek 2013-10-16 09:49:44 UTC
GCC 4.8.2 has been released.
Comment 11 Richard Biener 2014-05-22 09:05:53 UTC
GCC 4.8.3 is being released, adjusting target milestone.
Comment 12 Oleg Endo 2014-09-13 16:03:04 UTC
This issue should be revisited after the switch to LRA.
Comment 13 Jakub Jelinek 2014-12-19 13:27:29 UTC
GCC 4.8.4 has been released.
Comment 14 Richard Biener 2015-06-23 08:16:55 UTC
The gcc-4_8-branch is being closed, re-targeting regressions to 4.9.3.
Comment 15 Jakub Jelinek 2015-06-26 19:53:25 UTC
GCC 4.9.3 has been released.
Comment 16 Richard Biener 2016-08-03 10:58:14 UTC
GCC 4.9 branch is being closed
Comment 17 Jakub Jelinek 2018-10-26 10:21:20 UTC
GCC 6 branch is being closed
Comment 18 Richard Biener 2019-11-14 07:51:03 UTC
The GCC 7 branch is being closed, re-targeting to GCC 8.4.
Comment 19 Jakub Jelinek 2020-03-04 09:33:05 UTC
GCC 8.4.0 has been released, adjusting target milestone.
Comment 20 Jakub Jelinek 2021-05-14 09:46:42 UTC
GCC 8 branch is being closed.
Comment 21 Richard Biener 2021-06-01 08:05:42 UTC
GCC 9.4 is being released, retargeting bugs to GCC 9.5.
Comment 22 Richard Biener 2022-05-27 09:34:42 UTC
GCC 9 branch is being closed
Comment 23 Jakub Jelinek 2022-06-28 10:30:21 UTC
GCC 10.4 is being released, retargeting bugs to GCC 10.5.
Comment 24 Richard Biener 2023-07-07 10:29:47 UTC
GCC 10 branch is being closed.
Comment 25 Richard Biener 2024-07-19 12:55:56 UTC
GCC 11 branch is being closed.