[avr-gcc-list] Re: gcc 4.3.3 vs inline

[David Brown]

Szikra Istvan wrote:
> /**
> Category: miss optimization
> affects: avr-gcc 4.3.3
> Size optimization inline check generates bigger code
>
> I get this warning: "inlining failed in call to 'extIntSetEnable':
> optimizing for size and code size would grow"
> NO! It would not!
>
> Is this a known bug, or something that should be reported?
> (tried google 'avr optimizing for size and code size would grow' with no joy)

The algorithms for estimating code growth size on inlining are not 
perfect - it's a very hard problem to solve accurately while keeping a 
sensible compile time.  So much is done based on the complexity of the 
inline candidate by itself, not within the context of where it is used. 
 In cases like this where constant folding will reduce the inlined 
function to a few statements, you should use 
__attribute__((always_inline)) to force inlining.  Then you'll get the 
code you want.

mvh.,

David


> See the test case for an example:
> (sorry if it's still a bit much, but I tried to strip everything
> that's not relevant.
> I spent a lot of time on it, and it's a fraction of the original code.)
> In this case NOT inlining generates more then 5 times as much
> instructions than inlining!
> */
>
> #define extIntSetEnable_gcc_4_3_3_aail_workaround
> //#define extIntSetEnable_gcc_4_3_3_aail_workaround     
> __attribute__((always_inline))
>
> #include <avr/io.h>
> #include <avr/interrupt.h>
>
> #define BIT(x) (0x01 << (x))
> #define bit_set(p,b) ((p) |= BIT(b))
> #define bit_clear(p,b) ((p) &= ~BIT(b))
> //#define bit_write(p,b,c) (c ? bit_set(p,b) : bit_clear(p,b))
> #define bit_write(p,b,c) if (c) { bit_set(p,b);}else bit_clear(p,b)
>
> static inline
> void extIntSetEnable_gcc_4_3_3_aail_workaround extIntSetEnable(char
> channel, char enabled)
> {
>   switch (channel) { /// enable / disable int
>     case 0: {
>       bit_write(GICR,INT0,enabled);
>     } break;
>     case 1: {
>       bit_write(GICR,INT1,enabled);
>     } break;
>     case 2: {
>       bit_write(GICR,INT2,enabled);
>     } break;
>   }
> }
> int main (void)
> {
>   extIntSetEnable(0,0);
>   ///...
>   extIntSetEnable(0,1);
>   for (;;)
>   {
>   }
> }
>
> /**
> avr-gcc (GCC) 3.4.6
> avr-gcc -c -mmcu=atmega16 -I. -gdwarf-2 -DF_CPU=16000000UL
> -DBOOTSIZE=1024  -Os -funsigned-char -funsigned-bitfields
> -fpack-struct -fshort-enums -fno-strict-aliasing -Wall -Winline
> -Wstrict-prototypes -Wa,-adhlns=test_il.lst -ID:/Lib/ -std=gnu99 -MD
> -MP -MF .dep/test_il.o.d test_il.c -o test_il.o
> avr-gcc -mmcu=atmega16 -I. -gdwarf-2 -DF_CPU=16000000UL
> -DBOOTSIZE=1024  -Os -funsigned-char -funsigned-bitfields
> -fpack-struct -fshort-enums -fno-strict-aliasing -Wall -Winline
> -Wstrict-prototypes -Wa,-adhlns=test_il.o -ID:/Lib/ -std=gnu99 -MD -MP
> -MF .dep/test_il.elf.d test_il.o --output test_il.elf
> -Wl,-Map=test_il.map,--cref    -lm
> -Wl,--section-start=.bootloader=0x3800
> Size after:
> .text              184      0
>
> 00000054 <__ctors_end>:
>   58:   cf e5           ldi     r28, 0x5F       ; 95
>   5a:   d4 e0           ldi     r29, 0x04       ; 4
>   5c:   de bf           out     0x3e, r29       ; 62
>   5e:   cd bf           out     0x3d, r28       ; 61
> ...
> 0000008e <main>:
>   8e:   cf e5           ldi     r28, 0x5F       ; 95  <<< again? ... gcc 3.4.6 
> has
> it's own faults
>   90:   d4 e0           ldi     r29, 0x04       ; 4
>   92:   de bf           out     0x3e, r29       ; 62
>   94:   cd bf           out     0x3d, r28       ; 61
>
>   extIntSetEnable(0,0);
>   96:   8b b7           in      r24, 0x3b       ; 59
>   98:   8f 7b           andi    r24, 0xBF       ; 191
>   9a:   8b bf           out     0x3b, r24       ; 59
>   extIntSetEnable(0,1);
>   9c:   8b b7           in      r24, 0x3b       ; 59
>   9e:   80 64           ori     r24, 0x40       ; 64
>   a0:   8b bf           out     0x3b, r24       ; 59
>
> */
>
> /**
> avr-gcc (WinAVR 20090313) 4.3.2
> avr-gcc -c -mmcu=atmega16 -I. -gdwarf-2 -DF_CPU=16000000UL
> -DBOOTSIZE=1024  -Os -funsigned-char -funsigned-bitfields
> -fpack-struct -fshort-enums -fno-strict-aliasing
> -fno-inline-small-functions  -save-temps  -Wall -Winline
> -Wstrict-prototypes -Wa,-adhlns=test_il.lst -ID:/Lib/ -std=gnu99 -MD
> -MP -MF .dep/test_il.o.d test_il.c -o test_il.o
> avr-gcc -mmcu=atmega16 -I. -gdwarf-2 -DF_CPU=16000000UL
> -DBOOTSIZE=1024  -Os -funsigned-char -funsigned-bitfields
> -fpack-struct -fshort-enums -fno-strict-aliasing
> -fno-inline-small-functions  -save-temps  -Wall -Winline
> -Wstrict-prototypes -Wa,-adhlns=test_il.o -ID:/Lib/ -std=gnu99 -MD -MP
> -MF .dep/test_il.elf.d test_il.o --output test_il.elf
> -Wl,-Map=test_il.map,--cref    -lm
> -Wl,--section-start=.bootloader=0x3800
> Size after:
> .text              146      0
>
> 0000006c <main>:
>       bit_write(GICR,INT0,enabled);
>   6c:   8b b7           in      r24, 0x3b       ; 59
>   6e:   8f 7b           andi    r24, 0xBF       ; 191
>   70:   8b bf           out     0x3b, r24       ; 59
>
>   72:   8b b7           in      r24, 0x3b       ; 59
>   74:   80 64           ori     r24, 0x40       ; 64
>   76:   8b bf           out     0x3b, r24       ; 59
>
> */
>
> /**
> avr-gcc (WinAVR 20100110) 4.3.3
> avr-gcc -c -mmcu=atmega16 -I. -gdwarf-2 -DF_CPU=16000000UL
> -DBOOTSIZE=1024  -Os -funsigned-char -funsigned-bitfields
> -fpack-struct -fshort-enums -fno-strict-aliasing
> -fno-inline-small-functions  -save-temps  -Wall -Winline
> -Wstrict-prototypes -Wa,-adhlns=test_il.lst -ID:/Lib/ -std=gnu99 -MD
> -MP -MF .dep/test_il.o.d test_il.c -o test_il.o
> test_il.c: In function 'main':
> test_il.c:32: warning: inlining failed in call to 'extIntSetEnable':
> optimizing for size and code size would grow
> test_il.c:49: warning: called from here
> test_il.c:32: warning: inlining failed in call to 'extIntSetEnable':
> optimizing for size and code size would grow
> test_il.c:54: warning: called from here
> avr-gcc -mmcu=atmega16 -I. -gdwarf-2 -DF_CPU=16000000UL
> -DBOOTSIZE=1024  -Os -funsigned-char -funsigned-bitfields
> -fpack-struct -fshort-enums -fno-strict-aliasing
> -fno-inline-small-functions  -save-temps  -Wall -Winline
> -Wstrict-prototypes -Wa,-adhlns=test_il.o -ID:/Lib/ -std=gnu99 -MD -MP
> -MF .dep/test_il.elf.d test_il.o --output test_il.elf
> -Wl,-Map=test_il.map,--cref    -lm
> -Wl,--section-start=.bootloader=0x3800
> Size after:
> .text              216      0
>
>
> 0000006c <extIntSetEnable>:
>     switch (channel) { /// enable / disable int
>   6c:   81 30           cpi     r24, 0x01       ; 1
>   6e:   69 f0           breq    .+26            ; 0x8a <extIntSetEnable+0x1e>
>   70:   81 30           cpi     r24, 0x01       ; 1
>   72:   18 f0           brcs    .+6             ; 0x7a <extIntSetEnable+0xe>
>   74:   82 30           cpi     r24, 0x02       ; 2
>   76:   e1 f4           brne    .+56            ; 0xb0 <extIntSetEnable+0x44>
>   78:   10 c0           rjmp    .+32            ; 0x9a <extIntSetEnable+0x2e>
>     case 0: {
>       bit_write(GICR,INT0,enabled);
>   7a:   66 23           and     r22, r22
>   7c:   19 f0           breq    .+6             ; 0x84 <extIntSetEnable+0x18>
>   7e:   8b b7           in      r24, 0x3b       ; 59
>   80:   80 64           ori     r24, 0x40       ; 64
>   82:   0f c0           rjmp    .+30            ; 0xa2 <extIntSetEnable+0x36>
>   84:   8b b7           in      r24, 0x3b       ; 59
>   86:   8f 7b           andi    r24, 0xBF       ; 191
>   88:   0c c0           rjmp    .+24            ; 0xa2 <extIntSetEnable+0x36>
>     } break;
>     case 1: {
>       bit_write(GICR,INT1,enabled);
>   8a:   66 23           and     r22, r22
>   8c:   19 f0           breq    .+6             ; 0x94 <extIntSetEnable+0x28>
>   8e:   8b b7           in      r24, 0x3b       ; 59
>   90:   80 68           ori     r24, 0x80       ; 128
>   92:   07 c0           rjmp    .+14            ; 0xa2 <extIntSetEnable+0x36>
>   94:   8b b7           in      r24, 0x3b       ; 59
>   96:   8f 77           andi    r24, 0x7F       ; 127
>   98:   04 c0           rjmp    .+8             ; 0xa2 <extIntSetEnable+0x36>
>     } break;
>     case 2: {
>       bit_write(GICR,INT2,enabled);
>   9a:   66 23           and     r22, r22
>   9c:   21 f0           breq    .+8             ; 0xa6 <extIntSetEnable+0x3a>
>   9e:   8b b7           in      r24, 0x3b       ; 59
>   a0:   80 62           ori     r24, 0x20       ; 32
>   a2:   8b bf           out     0x3b, r24       ; 59
>   a4:   08 95           ret
>   a6:   8b b7           in      r24, 0x3b       ; 59
>   a8:   8f 7d           andi    r24, 0xDF       ; 223
>   aa:   8b bf           out     0x3b, r24       ; 59
>   ac:   08 95           ret
>
> 000000ae <main>:
>     extIntSetEnable(0,0);
>   ae:   80 e0           ldi     r24, 0x00       ; 0
>   b0:   60 e0           ldi     r22, 0x00       ; 0
>   b2:   0e 94 36 00     call    0x6c    ; 0x6c <extIntSetEnable>
>     extIntSetEnable(0,1);
>   b6:   80 e0           ldi     r24, 0x00       ; 0
>   b8:   61 e0           ldi     r22, 0x01       ; 1
>   ba:   0e 94 36 00     call    0x6c    ; 0x6c <extIntSetEnable>
>
> */
>
> SIGNAL(SIG_INTERRUPT0)
> {
> }