Bug 56791 - [4.8/4.9 Regression] Segmentation fault in stage2 gengenrtl -- Incorrect instruction sequence generated by reload
[4.8/4.9 Regression] Segmentation fault in stage2 gengenrtl -- Incorrect inst...
Status: RESOLVED FIXED
Product: gcc
Classification: Unclassified
Component: middle-end
4.8.0
: P2 normal
: 4.8.3
Assigned To: Not yet assigned to anyone
: build, wrong-code
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-31 00:57 UTC by John David Anglin
Modified: 2014-01-16 20:52 UTC (History)
2 users (show)

See Also:
Host: hppa1.1-hp-hp10.20
Target: hppa1.1-hp-hp10.20
Build: hppa1.1-hp-hp10.20
Known to work:
Known to fail:
Last reconfirmed:


Attachments
Preprocessed source (87.48 KB, text/plain)
2013-03-31 00:57 UTC, John David Anglin
Details
Candidate patch. (1.47 KB, patch)
2013-10-16 19:00 UTC, Bernd Schmidt
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John David Anglin 2013-03-31 00:57:54 UTC
Created attachment 29757 [details]
Preprocessed source

/xxx/gnu/gcc/objdir/./prev-gcc/xg++ -B/xxx/gnu/gcc/objdir/./prev-gcc/ -B/opt/gnu/gcc/gcc-4.8/hppa1.1-hp-hpux10.20/bin/ -nostdinc++ -B/xxx/gnu/gcc/objdir/prev-hppa1.1-hp-hpux10.20/libstdc++-v3/src/.libs -B/xxx/gnu/gcc/objdir/prev-hppa1.1-hp-hpux10.20/libstdc++-v3/libsupc++/.libs -I/xxx/gnu/gcc/objdir/prev-hppa1.1-hp-hpux10.20/libstdc++-v3/include/hppa1.1-hp-hpux10.20 -I/xxx/gnu/gcc/objdir/prev-hppa1.1-hp-hpux10.20/libstdc++-v3/include -I/xxx/gnu/gcc/gcc/libstdc++-v3/libsupc++ -L/xxx/gnu/gcc/objdir/prev-hppa1.1-hp-hpux10.20/libstdc++-v3/src/.libs -L/xxx/gnu/gcc/objdir/prev-hppa1.1-hp-hpux10.20/libstdc++-v3/libsupc++/.libs   -g -O2 -DIN_GCC   -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings   -DHAVE_CONFIG_H -DGENERATO
R_FILE -static-libstdc++ -static-libgcc  -o build/gengenrtl \
    build/gengenrtl.o build/errors.o .././libiberty/libiberty.a
build/gengenrtl > tmp-genrtl.h
/bin/sh: 9799 Memory fault(coredump)
make[3]: *** [s-genrtl-h] Error 139

In the generated assembly code, the fault occurs in the ldw instruction
using register %r19.

        .CALL ARGW0=GR
        bl puts,%r2
        ldo RR'L$C0034(%r26),%r26
L$BE0063:
L$BE0067:
        .stabn  68,0,270,L$M0084-L$FBB0002
L$M0084:
        ldw 0(%r19),%r20
        ldws,ma 4(%r20),%r4

%r19 is clobbered by the preceding call to puts.  The problem occurs
in reload.  Prior to reload, we have the following for insn 448:

(insn 448 447 450 57 (set (reg/v/f:SI 242 [ format ])
        (mem/f:SI (post_inc:SI (reg/f:SI 243 [ ivtmp.101 ])) [3 MEM[base: _104, 
offset: 0B]+0 S4 A32])) ../../gcc/gcc/gengenrtl.c:270 40 {*pa.md:2211}
     (expr_list:REG_INC (reg/f:SI 243 [ ivtmp.101 ])
        (nil)))

After reload, we have:

(call_insn 447 446 1092 57 (parallel [            (set (reg:SI 28 %r28)
                (call (mem:SI (symbol_ref/v:SI ("@puts") [flags 0x41]  <function
_decl 7ae76100 __builtin_puts>) [0 __builtin_puts S4 A32])
                    (const_int 16 [0x10])))
            (clobber (reg:SI 1 %r1))
            (clobber (reg:SI 2 %r2))
            (use (const_int 0 [0]))
        ]) ../../gcc/gcc/gengenrtl.c:247 203 {call_val_symref}
     (nil)
    (expr_list:REG_CC_SETTER (use (reg:SI 26 %r26))
        (nil)))
(insn 1092 447 1091 57 (set (reg:SI 20 %r20)
        (mem/c:SI (reg:SI 19 %r19) [7 %sfp+8 S4 A32])) ../../gcc/gcc/gengenrtl.c
:270 40 {*pa.md:2211}
     (nil))
(insn 1091 1092 448 57 (set (reg:SI 19 %r19)
        (plus:SI (reg/f:SI 30 %r30)
            (const_int -120 [0xffffff88]))) ../../gcc/gcc/gengenrtl.c:270 112 {a
ddsi3}
     (nil))
(insn 448 1091 1093 57 (set (reg/v/f:SI 4 %r4 [orig:242 format ] [242])
        (mem/f:SI (post_inc:SI (reg:SI 20 %r20)) [3 MEM[base: _104, offset: 0B]+0 S4 A32])) ../../gcc/gcc/gengenrtl.c:270 40 {*pa.md:2211}
     (expr_list:REG_INC (reg:SI 20 %r20)
        (nil)))

Insns 1091 and 1092 are interchanged.

I'm not sure but there appears to be special handling for increment instructions here:

  if (old != 0
      /* AUTO_INC reloads need to be handled even if inherited.  We got an
         AUTO_INC reload if reload_out is set but reload_out_reg isn't.  */
      && (! reload_inherited[j] || (rl->out && ! rl->out_reg))
      && ! rtx_equal_p (reg_rtx, old)
      && reg_rtx != 0)
    emit_input_reload_insns (chain, rld + j, old, j);

emit_input_reload_insns is called from the above after insn 1091 is emitted.

Can reproduce on i686-apple-darwin9.

Probably, this affects other hppa targets.

Compilation command:

-fpreprocessed gengenrtl.ii -quiet -dumpbase gengenrtl.c -mschedule=7100LC -auxbase-strip build/gengenrtl.o -g -O2 -Wextra -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wsuggest-attribute=format -Wpedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -version -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -o gengenrtl.s
Comment 1 John David Anglin 2013-04-09 23:17:04 UTC
Introduced in r195702.  Guess I didn't do enough testing of PA 1.1.
Looks like we need to ban symbolic destinations to work around this
bug.
Comment 2 Jakub Jelinek 2013-05-31 10:57:46 UTC
GCC 4.8.1 has been released.
Comment 3 John David Anglin 2013-07-28 18:52:14 UTC
I think this issue was latent and exposed by r195702.  It didn't change
the way auto increment/decrement instructions were handled.

Problem can be worked around on hppa1.1 as follows:

Index: pa.c
===================================================================
--- pa.c	(revision 198081)
+++ pa.c	(working copy)
@@ -513,6 +513,12 @@
       write_symbols = NO_DEBUG;
     }
 
+#ifdef AUTO_INC_DEC
+  /* FIXME: Disable auto increment and decrement processing until reload
+     is completed.  See PR middle-end 56791.  */
+  flag_auto_inc_dec = reload_completed;
+#endif
+
   /* We only support the "big PIC" model now.  And we always generate PIC
      code when in 64bit mode.  */
   if (flag_pic == 1 || TARGET_64BIT)

Problem can duplicated on hpux11 by configuring for hppa1.1.

However, I can't see any way to fix this in the PA backend.
Comment 4 John David Anglin 2013-09-20 23:58:45 UTC
Author: danglin
Date: Fri Sep 20 23:58:43 2013
New Revision: 202807

URL: http://gcc.gnu.org/viewcvs?rev=202807&root=gcc&view=rev
Log:
	PR middle-end/56791
	* config/pa/pa.c (pa_option_override): Disable auto increment and
	decrement instructions until reload is completed.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/pa/pa.c
Comment 5 John David Anglin 2013-09-21 00:00:39 UTC
Author: danglin
Date: Sat Sep 21 00:00:38 2013
New Revision: 202808

URL: http://gcc.gnu.org/viewcvs?rev=202808&root=gcc&view=rev
Log:
	PR middle-end/56791
	* config/pa/pa.c (pa_option_override): Disable auto increment and
	decrement instructions until reload is completed.


Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/pa/pa.c
Comment 6 Jakub Jelinek 2013-10-16 09:48:23 UTC
GCC 4.8.2 has been released.
Comment 7 Bernd Schmidt 2013-10-16 19:00:40 UTC
Created attachment 31021 [details]
Candidate patch.

The workaround patch is nonsensical since pa_option_override doesn't ever get called at the right times for this to work. The effect is to entirely disable autoinc generation.

Instead, try this. I think I'm at about 95% certainty that I understand the problem. The patch is however untested except to verify the output for the testcase looks plausible.
Comment 8 dave.anglin 2013-10-16 19:18:44 UTC
On 10/16/2013 3:00 PM, bernds at gcc dot gnu.org wrote:
> Candidate patch.
Thanks very much for looking at this bug.  Will test change shortly.
>
> The workaround patch is nonsensical since pa_option_override doesn't ever get
> called at the right times for this to work. The effect is to entirely disable
> autoinc generation.
I wasn't worried about that.  Autoinc generation is disabled for PA8000 
processors since
it's supposed to be unprofitable.  It's also disabled by default on 
Linux target.

I don't know the history of this and don't trust comment.  It might be 
that it was just broken.

Dave
Comment 9 John David Anglin 2013-12-31 18:08:32 UTC
Any chance the candidate patch can be submitted?
Comment 10 Bernd Schmidt 2014-01-03 12:46:49 UTC
(In reply to John David Anglin from comment #9)
> Any chance the candidate patch can be submitted?

I guess this means you've tested it and it works?
Comment 11 dave.anglin 2014-01-03 15:52:40 UTC
On 3-Jan-14, at 7:46 AM, bernds at gcc dot gnu.org wrote:

> I guess this means you've tested it and it works?

Yes, I have tested it and it works fine.

Dave
--
John David Anglin	dave.anglin@bell.net
Comment 12 Jeffrey A. Law 2014-01-07 20:47:11 UTC
autoincrement addressing modes take up two slots in the reorder buffers on the PA8000 CPUs and neither slot can be freed until both operations are retired.

By avoiding autoincrement addressing modes, the increments could proceed through to retirement independently of the memory reference, thus freeing an entry in the reorder buffer.  For the PA8000, disabling autoinc addressing modes was strictly a performance tweak, it wasn't used to paper over any bugs.
Comment 13 Jeffrey A. Law 2014-01-16 20:51:56 UTC
Author: law
Date: Thu Jan 16 20:51:24 2014
New Revision: 206688

URL: http://gcc.gnu.org/viewcvs?rev=206688&root=gcc&view=rev
Log:
014-01-16  Bernd Schmidt  <bernds@codesourcery.com>

        PR middle-end/56791
        * reload.c (find_reloads_address_1): Do not use RELOAD_OTHER
        * when
        pushing a reload for an autoinc when we had previously reloaded an
        inner part of the address.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/reload.c
Comment 14 Jeffrey A. Law 2014-01-16 20:52:55 UTC
I committed Bernd's patch which should fix this problem.