[Bug middle-end/27663] missed-optimization transforming a byte array to unsigned long

[Bug middle-end/27663] missed-optimization transforming a byte array to unsigned long

[gjl at gcc dot gnu.org]

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

--- Comment #6 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2011-05-16 14:20:25 UTC ---
Author: gjl
Date: Mon May 16 14:20:19 2011
New Revision: 173792

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=173792
Log:
    PR target/27663
    PR target/41076
    * config/avr/predicates.md (const_8_16_24_operand): New predicate.
    * config/avr/avr.md ("*ior<mode>qi.byte0",
    "*ior<mode>qi.byte1-3"): New define_insn_and_split patterns.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/avr/avr.md
    trunk/gcc/config/avr/predicates.md

[Bug middle-end/27663] missed-optimization transforming a byte array to unsigned long

[gjl at gcc dot gnu.org]

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

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gjl at gcc dot gnu.org
      Known to fail|                            |

--- Comment #7 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2011-05-16 15:05:41 UTC ---
The patch tries to fix the middle-end flaw in the BE by introducing some
combine patterns that recognize byte-insert.

Wouldn't it be possible to recognize such situations in the middle-end and map
them to something like (set (zero_extract:QI (reg:SI) ...)) or (set (subreg:QI
(reg:SI) ...)?

Even if the bytes inserted do not come from consecutive memory locations, such
a recognition would help.

The patch does not lead to optimal code, there is still room for improvement:

With -Os -mmcu=atmega8:

f:
    push r16
    push r17
/* prologue: function */
/* frame size = 0 */
/* stack size = 2 */
.L__stack_usage = 2
    movw r30,r24
    ldd r24,Z+1
    ldd r16,Z+2
    ldi r17,lo8(0)
    ldi r18,lo8(0)
    ldi r19,hi8(0)
    movw r18,r16
    clr r17
    clr r16
    or r19,r24
    ldd r24,Z+4
    or r16,r24
    ldd r24,Z+3
    or r17,r24
    movw r22,r16
    movw r24,r18
/* epilogue start */
    pop r17
    pop r16
    ret

The usage of r16/r17 might be an artifact of IRA because only half of a SI reg
is call-saved, the other half is call-used. There is the following comment in
ira-color.c:

    /* We need to save/restore the hard register in
       epilogue/prologue.  Therefore we increase the cost.  */
    {
      /* ??? If only part is call clobbered.  */

Despite subreg lowering, the call-used r26/r27 are not used.

Maybe you should also try to disable subreg lowering by means of
-fno-split-wide-types. For the code in question that gives:

With -Os -mmcu=atmega8 -fno-split-wide-types:

f:
/* prologue: function */
/* frame size = 0 */
/* stack size = 0 */
.L__stack_usage = 0
    movw r30,r24
    ldd r18,Z+1
    ldd r22,Z+2
    mov r24,r22
    ldi r25,lo8(0)
    ldi r26,lo8(0)
    ldi r27,hi8(0)
    clr r23
    clr r22
    or r25,r18
    ldd r18,Z+4
    or r22,r18
    ldd r18,Z+3
    or r23,r18
/* epilogue start */
    ret

What I do not understand are the insns clearing r26/r27 because they are dead
(which is not detected). It is an HI insn that looks like that:

; (insn 32 34 42 (set (reg:HI 26 r26 [ MEM[(unsigned char *)P_1(D) + 2B]+2 ])
;         (const_int 0 [0])) insert-byte.c:5 10 {*movhi}
;      (nil))
    ldi r26,lo8(0)     ;  32    *movhi/1    [length = 2]
    ldi r27,hi8(0)

[Bug middle-end/27663] missed-optimization transforming a byte array to unsigned long

[jackie.rosen at hushmail dot com]

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

Jackie Rosen <jackie.rosen at hushmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jackie.rosen at hushmail dot com

--- Comment #8 from Jackie Rosen <jackie.rosen at hushmail dot com> ---
*** Bug 260998 has been marked as a duplicate of this bug. ***
Seen from the domain http://volichat.com
Page where seen: http://volichat.com/adult-chat-rooms
Marked for reference. Resolved as fixed @bugzilla.

[Bug target/27663] missed-optimization transforming a byte array to unsigned long

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=27663

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2007-07-25 17:29:16         |2023-5-30
          Component|middle-end                  |target
      Known to fail|                            |5.1.0

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Starting around GCC 5 or so, a call to __bswapsi2 is done here.

my bet is if avr target adds a bswapsi2 pattern (which either expands or splits
into the best moves), this will be optimized correctly.

[Bug target/27663] missed-optimization transforming a byte array to unsigned long

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=27663

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |14.0
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #10 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
bswap recognition appears to have improved since then.  When I -Os
-mmcu=atmega8 -dp with 14.0.0 20230530:

unsigned long f_mem1 (unsigned char *P)
{
  unsigned long c;
  c = ((unsigned long) P[1] << 24)
    | ((unsigned long) P[2] << 16)
    | ((unsigned long) P[3] << 8)
    | ((unsigned long) P[4] << 0);
  return c;
}

unsigned long f_mem2 (unsigned char *P)
{
  unsigned long c;
  c = ((unsigned long) P[4] << 0)
    | ((unsigned long) P[3] << 8)
    | ((unsigned long) P[2] << 16)
    | ((unsigned long) P[1] << 24);
  return c;
}

unsigned long f_reg (unsigned long p)
{
  unsigned long c;
  c = (p << 24)
    | (0xff0000 & (p << 8))
    | (0xff00 & (p >> 8))
    | (p >> 24);
  return c;
}


I am getting:


f_mem1:
        movw r30,r24     ;  33  [c=4 l=1]  *movhi/0
        ldd r22,Z+1      ;  34  [c=16 l=4]  *movsi/2
        ldd r23,Z+2
        ldd r24,Z+3
        ldd r25,Z+4
        rcall __bswapsi2         ;  35  [c=16 l=1]  *bswapsi2.libgcc
        ret              ;  38  [c=0 l=1]  return

f_mem2:
        movw r30,r24     ;  33  [c=4 l=1]  *movhi/0
        ldd r22,Z+1      ;  34  [c=16 l=4]  *movsi/2
        ldd r23,Z+2
        ldd r24,Z+3
        ldd r25,Z+4
        rcall __bswapsi2         ;  35  [c=16 l=1]  *bswapsi2.libgcc
        ret              ;  38  [c=0 l=1]  return

f_reg:
        rcall __bswapsi2         ;  42  [c=16 l=1]  *bswapsi2.libgcc
        ret              ;  45  [c=0 l=1]  return

The RCALL + RET will be optimized to RJMP by -mrelax.

Splitting into a zoo of subregs is usually not a good idea because the register
allocator won't manage to optimize them, resulting in bunch of moves and
register pressure that are more expensive than the bswap itself.

Notice the calls are transparent, i.e. their footprint is just as big as
required:

(define_insn "*bswapsi2.libgcc"
  [(set (reg:SI 22)
        (bswap:SI (reg:SI 22)))
   (clobber (reg:CC REG_CC))]
  "reload_completed"
  "%~call __bswapsi2"
  [(set_attr "type" "xcall")])

[Bug target/27663] missed-optimization transforming a byte array to unsigned long

[gjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=27663

--- Comment #11 from Georg-Johann Lay <gjl at gcc dot gnu.org> ---
(In reply to Georg-Johann Lay from comment #10)
> bswap recognition appears to have improved since then [...]

Closed thusly.

[Bug target/27663] missed-optimization transforming a byte array to unsigned long

[urjaman at gmail dot com]

------- Comment #3 from urjaman at gmail dot com  2009-04-24 20:24 -------
Confirmed on 4.3.2 - it's a bit different and actually worse (longer):
(Please add 4.3.2 to known to fail - i cant)
00000000 <f>:
   0:   e8 2f           mov     r30, r24
   2:   f9 2f           mov     r31, r25
   4:   21 81           ldd     r18, Z+1        ; 0x01
   6:   30 e0           ldi     r19, 0x00       ; 0
   8:   40 e0           ldi     r20, 0x00       ; 0
   a:   50 e0           ldi     r21, 0x00       ; 0
   c:   52 2f           mov     r21, r18
   e:   44 27           eor     r20, r20
  10:   33 27           eor     r19, r19
  12:   22 27           eor     r18, r18
  14:   82 81           ldd     r24, Z+2        ; 0x02
  16:   90 e0           ldi     r25, 0x00       ; 0
  18:   a0 e0           ldi     r26, 0x00       ; 0
  1a:   b0 e0           ldi     r27, 0x00       ; 0
  1c:   a8 2f           mov     r26, r24
  1e:   b9 2f           mov     r27, r25
  20:   99 27           eor     r25, r25
  22:   88 27           eor     r24, r24
  24:   28 2b           or      r18, r24
  26:   39 2b           or      r19, r25
  28:   4a 2b           or      r20, r26
  2a:   5b 2b           or      r21, r27
  2c:   84 81           ldd     r24, Z+4        ; 0x04
  2e:   90 e0           ldi     r25, 0x00       ; 0
  30:   a0 e0           ldi     r26, 0x00       ; 0
  32:   b0 e0           ldi     r27, 0x00       ; 0
  34:   28 2b           or      r18, r24
  36:   39 2b           or      r19, r25
  38:   4a 2b           or      r20, r26
  3a:   5b 2b           or      r21, r27
  3c:   83 81           ldd     r24, Z+3        ; 0x03
  3e:   90 e0           ldi     r25, 0x00       ; 0
  40:   a0 e0           ldi     r26, 0x00       ; 0
  42:   b0 e0           ldi     r27, 0x00       ; 0
  44:   ba 2f           mov     r27, r26
  46:   a9 2f           mov     r26, r25
  48:   98 2f           mov     r25, r24
  4a:   88 27           eor     r24, r24
  4c:   28 2b           or      r18, r24
  4e:   39 2b           or      r19, r25
  50:   4a 2b           or      r20, r26
  52:   5b 2b           or      r21, r27
  54:   62 2f           mov     r22, r18
  56:   73 2f           mov     r23, r19
  58:   84 2f           mov     r24, r20
  5a:   95 2f           mov     r25, r21
  5c:   08 95           ret


-- 


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

[Bug target/27663] missed-optimization transforming a byte array to unsigned long

[eweddington at cso dot atmel dot com]

------- Comment #2 from eweddington at cso dot atmel dot com  2007-07-25 17:29 -------
Confirmed on 4.2.1.


-- 

eweddington at cso dot atmel dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
      Known to fail|                            |4.2.0 4.2.1
   Last reconfirmed|0000-00-00 00:00:00         |2007-07-25 17:29:16
               date|                            |


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

[Bug target/27663] missed-optimization transforming a byte array to unsigned long

[eweddington at cso dot atmel dot com]

------- Comment #1 from eweddington at cso dot atmel dot com  2007-04-09 23:33 -------
Bernd, what mcu type was this compiled for?


-- 


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