[Bug target/60039] New: sh3 optimisation bug with -O2

[Bug target/60039] New: sh3 optimisation bug with -O2

[skrll at netbsd dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60039

            Bug ID: 60039
           Summary: sh3 optimisation bug with -O2
           Product: gcc
           Version: 4.8.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: skrll at netbsd dot org

Created attachment 32020
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32020&action=edit
NetBSD's /usr/src/libexec/ld.elf_so/xmalloc

Compiling the attached file for NetBSD/sh3 results in incorrect code.

[Bug target/60039] sh3 optimisation bug with -O2

[skrll at netbsd dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60039

--- Comment #1 from Nick Hudson <skrll at netbsd dot org> ---
With -O2 I see invalid pointer derefs in the morecore function called by
imalloc

[Bug target/60039] sh3 optimisation bug with -O2

[skrll at netbsd dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60039

--- Comment #2 from Nick Hudson <skrll at netbsd dot org> ---
/wrk/obj/landisk/wrk/netbsd/gcc48x/src/tooldir.NetBSD-6.99.28-amd64/bin/shle--netbsdelf-gcc
-fvisibility=hidden -fstack-protector -Wstack-protector
-Wno-error=stack-protector   --param ssp-buffer-size=1  -g -std=gnu99  -Wall
-Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wno-sign-compare 
-Wno-traditional  -Wa,--fatal-warnings -Wreturn-type -Wswitch -Wshadow
-Wcast-qual -Wwrite-strings -Wextra -Wno-unused-parameter -Wno-sign-compare
-Wsign-compare -Wformat=2  -Wno-format-zero-length  -Wno-uninitialized
-Wno-maybe-uninitialized -Werror   
--sysroot=/wrk/obj/landisk/wrk/netbsd/gcc48x/src/destdir.landisk -fpic
-DELFSIZE=32 -DLIBDIR=\"/usr/lib\" -D_PATH_RTLD=\"/libexec/ld.elf_so\"
-I/wrk/netbsd/gcc48x/src/libexec/ld.elf_so -I. -DRTLD_LOADER -D_RTLD_SOURCE
-DCOMBRELOC  -D_FORTIFY_SOURCE=2 -c -g  -O2   /tmp/xmalloc.i -o
/tmp/xmalloc.O2.o

Compile line

[Bug target/60039] sh3 optimisation bug with -O2

[skrll at netbsd dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60039

--- Comment #3 from Nick Hudson <skrll at netbsd dot org> ---
 1b6:   93 60           mov     r9,r0
 1b8:   76 02           mov.l   r7,@(r0,r2)
        while (--nblks > 0) {
 1ba:   10 41           dt      r1
 1bc:   03 8f           bf.s    1c6 <imalloc+0xd6>
 1be:   73 62           mov     r7,r2
         * If nothing in hash bucket right now,
         * request more memory from the system.
         */
        if ((op = nextf[bucket]) == NULL) {
                morecore(bucket);
                if ((op = nextf[bucket]) == NULL)
 1c0:   08 a0           bra     1d4 <imalloc+0xe4>
 1c2:   28 22           tst     r2,r2
         * free list for this hash bucket.
         */
        nextf[bucket] = op;
        while (--nblks > 0) {
                op->ov_next = (union overhead *)((caddr_t)op + sz);
                op = (union overhead *)((caddr_t)op + sz);
 1c4:   33 62           mov     r3,r2
         * Add new memory allocated to that on
         * free list for this hash bucket.
         */
        nextf[bucket] = op;
        while (--nblks > 0) {
                op->ov_next = (union overhead *)((caddr_t)op + sz);
 1c6:   23 63           mov     r2,r3
 1c8:   dc 33           add     r13,r3
        /*
         * Add new memory allocated to that on
         * free list for this hash bucket.
         */
        nextf[bucket] = op;
        while (--nblks > 0) {
 1ca:   10 41           dt      r1
 1cc:   fa 8f           bf.s    1c4 <imalloc+0xd4>
 1ce:   32 22           mov.l   r3,@r2
 1d0:   73 62           mov     r7,r2

r2 at 0x1ce is somehow wrong

[Bug target/60039] sh3 optimisation bug with -O2

[skrll at netbsd dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60039

--- Comment #4 from Nick Hudson <skrll at netbsd dot org> ---
Created attachment 32076
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=32076&action=edit
Full assembler output

[Bug target/60039] sh3 optimisation bug with -O2

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60039

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kkojima at gcc dot gnu.org

--- Comment #5 from Oleg Endo <olegendo at gcc dot gnu.org> ---
On 4.9 r208592 and my newlib cross setup, I've tried compiling the
pre-processed source from attachment 32020 with '-std=gnu99 -O2 -m3 -ml -fpic'
and I get

        dt      r1
        bt.s    .L44
        mov     r7,r2

.L45:
        mov     r2,r3
        add     r13,r3   // r3 = op + sz
        mov.l   r3,@r2   // op->ov_next = ...
        dt      r1     // r1 = nblks - 1
        bf.s    .L45
        mov     r3,r2    // op = op + sz

        mov     r7,r2
.L44:
        tst     r2,r2
        bf      .L39

so the code is basically the same, just the mem store is placed before the
branch instead of the delay slot.  As far as I can see, that's what is written
in the C code.  Am I missing something?

[Bug target/60039] sh3 optimisation bug with -O2

[uwe at netbsd dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60039

--- Comment #6 from Valeriy E. Ushakov <uwe at netbsd dot org> ---
As far as I can tell the actual problem is in this code:

    .loc 1 189 0
    mov.l    .L91,r0
    ; ...
    add    r12,r0
    mov.l    .L84,r1
    jsr    @r0    ; udivsi3 for 32*1024/pagesz
     ; ...
    .loc 1 189 0
    bsrf    r1    ; morepages call
.LPCS6:
     mov    r0,r4    ; with result of division

.L91:
    .long    __udivsi3@GOTOFF
.L84:
    .long    morepages-(.LPCS6+2)

That code assumes that r1 is not clobbered, but in our case it is.
So instead of calling morepages() we end up with a small branch forward.
and pagepool_start is not initialized properly, causing SIGSEGV later.

I don't remember how we build udivsi3 &c bits in NetBSD, we might be
violating assumptions about clobbered regs.

FWIW, our in-tree gcc 4.5 generates for this:

        mov.l   .L75,r0
    ; ...
        mov.l   @(r0,r12),r2
        jsr     @r2
         ; ...
        mov.l   .L67,r1
        mov     r0,r4
        bsrf    r1
.LPCS6:

.L75:
        .long   __udivsi3_i4i@GOT
.L67:
        .long   morepages-(.LPCS6+2)

Because, as far as I can tell, in 4.5 sh_override_options() has an extra

    else if (TARGET_SH2)    /* ??? EXPERIMENTAL */
      sh_div_strategy = SH_DIV_CALL_TABLE;

clause, that is missing in 4.8 sh_option_override().

[Bug target/60039] sh3 optimisation bug with -O2

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60039

--- Comment #7 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Ugh, then this is an old problem and we've missed to give a correct
clobber information to udivsi3_i1 insn for PIC.  Does the patch
below fix the issue?

--- gcc/config/sh/sh.md.orig    2013-09-13 17:38:22.000000000 +0900
+++ gcc/config/sh/sh.md    2014-03-18 11:08:19.868887133 +0900
@@ -2152,6 +2152,7 @@
     (udiv:SI (reg:SI R4_REG) (reg:SI R5_REG)))
    (clobber (reg:SI T_REG))
    (clobber (reg:SI PR_REG))
+   (clobber (reg:SI R1_REG))
    (clobber (reg:SI R4_REG))
    (use (match_operand:SI 1 "arith_reg_operand" "r"))]
   "TARGET_SH1 && TARGET_DIVIDE_CALL_DIV1"

[Bug target/60039] sh3 optimisation bug with -O2

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60039

Kazumoto Kojima <kkojima at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #11 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
Fixed.