[Bug target/40603] unnecessary conversion from unsigned byte load to signed byte load

[Bug target/40603] unnecessary conversion from unsigned byte load to signed byte load

[carrot at google dot com]

------- Comment #1 from carrot at google dot com  2009-07-01 06:56 -------
Created an attachment (id=18105)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=18105&action=view)
test case


-- 


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

[Bug target/40603] New: unnecessary conversion from unsigned byte load to signed byte load

[carrot at google dot com]

Compile the following function with options -Os -mthumb -march=armv5te

int ldrb(unsigned char* p)
{
    if (p[8] <= 0x7F)
          return 2;
      else
            return 5;
}

Gcc generates following codes:

        push    {lr}
        mov     r3, #8
        ldrsb   r3, [r0, r3]
        mov     r0, #2
        cmp     r3, #0
        bge     .L2
        mov     r0, #5
.L2:
        @ sp needed for prologue
        pop     {pc}

The source code    if (p[8] <= 0x7F) is translated to:

        mov     r3, #8
        ldrsb   r3, [r0, r3]
        cmp     r3, #0

A better code sequence should be:

        ldrb    r3, [r0, 8]
        cmp     r3, 0x7F

This can save one instruction.

The tree dump shows in a very early pass (ldrb.c.003t.original) the comparison
was transformed to
       if ((signed char) *(p + 8) >= 0)

I guess gcc thinks comparing with 0 is much cheaper than comparing with other
numbers. Am I right?

Unfortunately in thumb mode, loading a signed byte costs more than loading an
unsigned byte and comparing with 0 has same cost as comparing with 0x7F.


-- 
           Summary: unnecessary conversion from unsigned byte load to signed
                    byte load
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: carrot at google dot com
 GCC build triplet: i686-linux
  GCC host triplet: i686-linux
GCC target triplet: arm-eabi


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

[Bug target/40603] unnecessary conversion from unsigned byte load to signed byte load

[ramana dot radhakrishnan at arm dot com]

------- Comment #2 from ramana dot radhakrishnan at arm dot com  2009-07-01 09:13 -------
Subject: Re:   New: unnecessary conversion from unsigned
        byte load to signed byte load


> Unfortunately in thumb mode, loading a signed byte costs more than loading an
> unsigned byte and comparing with 0 has same cost as comparing with 0x7F.

I don't know of any core where loading a signed byte is more expensive
than unsigned byte in thumb mode. What did you have in mind ?

I suspect what you mean is that the sign extension here is not required
and we could get away with ldrb.




> 


-- 


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

Re: [Bug target/40603] New: unnecessary conversion from unsigned byte load to signed byte load

[Ramana Radhakrishnan]

> Unfortunately in thumb mode, loading a signed byte costs more than loading an
> unsigned byte and comparing with 0 has same cost as comparing with 0x7F.

I don't know of any core where loading a signed byte is more expensive
than unsigned byte in thumb mode. What did you have in mind ?

I suspect what you mean is that the sign extension here is not required
and we could get away with ldrb.




> 

[Bug target/40603] unnecessary conversion from unsigned byte load to signed byte load

[carrot at google dot com]

------- Comment #3 from carrot at google dot com  2009-07-01 10:24 -------
(In reply to comment #2)
> Subject: Re:   New: unnecessary conversion from unsigned
>         byte load to signed byte load
> 
> 
> > Unfortunately in thumb mode, loading a signed byte costs more than loading an
> > unsigned byte and comparing with 0 has same cost as comparing with 0x7F.
> 
> I don't know of any core where loading a signed byte is more expensive
> than unsigned byte in thumb mode. What did you have in mind ?
> 
> I suspect what you mean is that the sign extension here is not required
> and we could get away with ldrb.
> 
In thumb1, instruction ldrb has an addressing mode of Rn + imm5, but ldrsb has
only addressing mode of Rn + Rm. So loading unsigned byte from p[8] needs only
one instruction
    ldrb r3, [r0, 8]

But loading singed byte from p[8] needs two instructions:

    mov   r3, 8
    ldrsb r3, [r0, r3]

So in this case (base + constant offset), loading a signed byte is more
expensive than unsigned byte in thumb mode.


-- 


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

[Bug target/40603] unnecessary conversion from unsigned byte load to signed byte load

[ramana at gcc dot gnu dot org]

-- 

ramana at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2009-07-08 10:49:52
               date|                            |


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

[Bug target/40603] unnecessary conversion from unsigned byte load to signed byte load

[rearnsha at gcc dot gnu dot org]

------- Comment #4 from rearnsha at gcc dot gnu dot org  2009-07-22 21:22 -------
The transformation to signed char is done very early on (maybe even during
parsing).  The 003t.original dump already contains:
  if ((signed char) *(p + 8) >= 0)


-- 

rearnsha at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rearnsha at gcc dot gnu dot
                   |                            |org


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

[Bug target/40603] unnecessary conversion from unsigned byte load to signed byte load

[ramana at gcc dot gnu dot org]

-- 

ramana at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
           Keywords|                            |missed-optimization


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

[Bug target/40603] unnecessary conversion from unsigned byte load to signed byte load

[bernds at codesourcery dot com]

------- Comment #5 from bernds at codesourcery dot com  2010-03-26 12:01 -------
http://gcc.gnu.org/ml/gcc-patches/2010-03/msg01235.html


-- 


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

[Bug target/40603] unnecessary conversion from unsigned byte load to signed byte load

[bernds at gcc dot gnu dot org]

------- Comment #6 from bernds at gcc dot gnu dot org  2010-04-16 10:05 -------
Subject: Bug 40603

Author: bernds
Date: Fri Apr 16 10:04:15 2010
New Revision: 158407

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=158407
Log:
        PR target/40603
        * config/arm/arm.md (cbranchqi4): New pattern.
        * config/arm/predicates.md (const0_operand,
        cbranchqi4_comparison_operator): New predicates.

        PR target/40603
        * gcc.target/arm/thumb-cbranchqi.c: New test.

Added:
    trunk/gcc/testsuite/gcc.target/arm/thumb-cbranchqi.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.md
    trunk/gcc/config/arm/predicates.md
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug target/40603] unnecessary conversion from unsigned byte load to signed byte load

[bernds at gcc dot gnu dot org]

------- Comment #7 from bernds at gcc dot gnu dot org  2010-04-16 11:23 -------
Fixed.


-- 

bernds at gcc dot gnu dot org changed:

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


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