[Bug target/17019] New: THUMB -O1: bad code generated for simple for loop

[Bug target/17019] New: THUMB -O1: bad code generated for simple for loop

[danfuzz at milk dot com]

Compiling the following with -O1 for thumb results in bogus code being generated:

    void badness(int a)
    {
        void zorch(int b);
        int b;

        for (b = -1; b < a + 1; b++) {
            zorch(b);
        }
    }

In particular, it looks like the loop prologue code is screwed up and will cause the loop to always be 
skipped.

Here's how I configured and built the compiler:

$ tar -xjvf gcc-core-3.4.0.tar.bz2
$ cd gcc-3.4.0
$ ./configure --prefix=/usr/local/armdev --target=arm-elf --with-newlib --enable-languages=c

The following lines were uncommented in gcc/config/arm/t-arm-elf:

MULTILIB_OPTIONS    += mno-thumb-interwork/mthumb-interwork
MULTILIB_DIRNAMES   += normal interwork
MULTILIB_EXCEPTIONS += *mapcs-26/*mthumb-interwork*

$ make
$ sudo make install

In case it matters, I'm currently using binutils-2.15.

I will try this with 3.4.1 shortly.

-- 
           Summary: THUMB -O1: bad code generated for simple for loop
           Product: gcc
           Version: 3.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: danfuzz at milk dot com
                CC: gcc-bugs at gcc dot gnu dot org
GCC target triplet: arm-unknown-elf


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

[Bug target/17019] THUMB -O1: bad code generated for simple for loop

[danfuzz at milk dot com]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code


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

[Bug target/17019] THUMB -O1: bad code generated for simple for loop

[danfuzz at milk dot com]

------- Additional Comments From danfuzz at milk dot com  2004-08-13 16:45 -------
Sorry, forgot to put in the explicit compile line. Here it is:

arm-elf-gcc -save-temps -mthumb -O1 -c blort.c


-- 


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

[Bug target/17019] THUMB -O1: bad code generated for simple for loop

[danfuzz at milk dot com]

------- Additional Comments From danfuzz at milk dot com  2004-08-13 17:18 -------
The bug still seems to happen with 3.4.1. For the record, here's the assemby code that I think is in 
error:

badness:
        push    {r4, r5, lr}
        mov     r4, #1
        neg     r4, r4
        // probably missing a test here
        bmi     .L7

That is, the code correctly initializes r4 to -1, but then always branches around the loop because the 
branch condition is always true.

-- 


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

[Bug target/17019] THUMB -O1: bad code generated for simple for loop

[danfuzz at milk dot com]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |3.4.0
            Version|3.4.0                       |3.4.1


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

[Bug target/17019] THUMB -O1: bad code generated for simple for loop

[danfuzz at milk dot com]

------- Additional Comments From danfuzz at milk dot com  2004-08-13 17:54 -------
I just looked at the intermediate RTL, and, near as I can tell, as of pass 35.mach, the code is ok. Here's 
the loop skip test:

(jump_insn 44 74 72 (parallel [
            (set (pc)
                (if_then_else (lt (plus:SI (reg/v:SI 0 r0 [orig:68 a ] [68])
                            (const_int 1 [0x1]))
                        (const_int 0 [0x0]))
                    (label_ref 45)
                    (pc)))
            (clobber (reg:SI 3 r3))
        ]) 176 {*addsi3_cbranch_scratch} (insn_list 3 (nil))
    (expr_list:REG_UNUSED (reg:SI 3 r3)
        (expr_list:REG_BR_PROB (const_int 3600 [0xe10])
            (nil))))

I read that as skipping the loop if (a + 1) < 0, which I believe is correct.

-- 


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

[Bug target/17019] THUMB -O1: bad code generated for simple for loop

[danfuzz at milk dot com]

------- Additional Comments From danfuzz at milk dot com  2004-08-13 20:07 -------
If I compile with -dP, I find that the bmi instruction gets annotated like this:

@(jump_insn 44 74 72 (parallel [
@            (set (pc)
@                (if_then_else (lt (plus:SI (reg/v:SI 0 r0 [orig:68 a ] [68])
@                            (const_int 1 [0x1]))
@                        (const_int 0 [0x0]))
@                    (label_ref 45)
@                    (pc)))
@            (clobber (reg:SI 3 r3))
@        ]) 176 {*addsi3_cbranch_scratch} (insn_list 3 (nil))
@    (expr_list:REG_UNUSED (reg:SI 3 r3)
@        (expr_list:REG_BR_PROB (const_int 3600 [0xe10])
@            (nil))))
@ 0x0004
        bmi     .L7     @ 44    *addsi3_cbranch_scratch/3       [length = 4]

This seems to correspond to arm.md around line 6211.

Assuming that that is indeed the right pattern to match, then it looks like either there's a missing case 
in the switch statement or the which_alternative variable got set incorrectly.

-- 


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

[Bug target/17019] THUMB -O1: bad code generated for simple for loop

[danfuzz at milk dot com]

------- Additional Comments From danfuzz at milk dot com  2004-08-13 20:11 -------
I wrote:
>Assuming that that is indeed the right pattern to match, then it looks like either there's a missing
>case in the switch statement or the which_alternative variable got set incorrectly.

I just checked, and it looks like which_alternative is 2 in this case, which is not one of the cases of the 
switch.


-- 


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

[Bug target/17019] THUMB -O1: bad code generated for simple for loop

[danfuzz at milk dot com]

------- Additional Comments From danfuzz at milk dot com  2004-08-13 21:23 -------
After giving myself a crash course on gcc md file syntax, I was convinced that the switch statement 
cases are actually supposed to be 0-3 as opposed to skipping 2. I made the change and it looks like the 
problem went away, at least from the test case I submitted with this bug.

-- 


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

[Bug target/17019] THUMB: bad switch statement in md code for addsi3_cbranch_scratch

[danfuzz at milk dot com]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|THUMB -O1: bad code         |THUMB: bad switch statement
                   |generated for simple for    |in md code for
                   |loop                        |addsi3_cbranch_scratch


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

[Bug target/17019] THUMB: bad switch statement in md code for addsi3_cbranch_scratch

[danfuzz at milk dot com]

------- Additional Comments From danfuzz at milk dot com  2004-08-13 21:36 -------
Created an attachment (id=6922)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=6922&action=view)
proposed patch


-- 


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

[Bug target/17019] THUMB: bad switch statement in md code for addsi3_cbranch_scratch

[rearnsha at gcc dot gnu dot org]

------- Additional Comments From rearnsha at gcc dot gnu dot org  2004-08-16 14:53 -------
I agree with your analysis.

I'm just running a test to see if this patch fixes another problem I was in the
early stages of trying to track down.

Thanks for the detailed bug report.


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2004-08-16 14:53:29
               date|                            |


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

[Bug target/17019] THUMB: bad switch statement in md code for addsi3_cbranch_scratch

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-08-17 10:01 -------
Subject: Bug 17019

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	rearnsha@gcc.gnu.org	2004-08-17 10:01:50

Modified files:
	gcc            : ChangeLog 
	gcc/config/arm : arm.md 

Log message:
	From Daniel Bornstein  <danfuzz@milk.com>
	PR target/17019
	* arm.md (addsi3_cbranch_scratch): Correct case labels.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.4932&r2=2.4933
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/arm/arm.md.diff?cvsroot=gcc&r1=1.176&r2=1.177



-- 


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

[Bug target/17019] THUMB: bad switch statement in md code for addsi3_cbranch_scratch

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-08-17 10:18 -------
Subject: Bug 17019

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	rearnsha@gcc.gnu.org	2004-08-17 10:18:32

Modified files:
	gcc            : ChangeLog 
	gcc/config/arm : arm.md 

Log message:
	From Daniel Bornstein  <danfuzz@milk.com>
	PR target/17019
	* arm.md (addsi3_cbranch_scratch): Correct case labels.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.582&r2=2.2326.2.583
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/arm/arm.md.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.154&r2=1.154.4.1



-- 


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

[Bug target/17019] THUMB: bad switch statement in md code for addsi3_cbranch_scratch

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-08-17 11:32 -------
Subject: Bug 17019

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	csl-arm-branch
Changes by:	rearnsha@gcc.gnu.org	2004-08-17 11:32:24

Modified files:
	gcc            : ChangeLog.csl-arm 
	gcc/config/arm : arm.md 

Log message:
	From Daniel Bornstein  <danfuzz@milk.com>
	PR target/17019
	* arm.md (addsi3_cbranch_scratch): Correct case labels.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.csl-arm.diff?cvsroot=gcc&only_with_tag=csl-arm-branch&r1=1.1.2.6&r2=1.1.2.7
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/arm/arm.md.diff?cvsroot=gcc&only_with_tag=csl-arm-branch&r1=1.145.2.24&r2=1.145.2.25



-- 


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

[Bug target/17019] THUMB: bad switch statement in md code for addsi3_cbranch_scratch

[rearnsha at gcc dot gnu dot org]

------- Additional Comments From rearnsha at gcc dot gnu dot org  2004-08-17 11:33 -------
I've applied the patch you proposed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |3.4.2


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