[Bug c/38629] New: gcc version 4.2.x, 4.3.x and 4.4.0 are doing (too much) automatic inlining with -O1, -O2 and -Os

[Bug c/38629] New: gcc version 4.2.x, 4.3.x and 4.4.0 are doing (too much) automatic inlining with -O1, -O2 and -Os

[cbm at d81 dot de]

Hello GCC bug hunters,

I'm using GCC C compiler, compiled as a cross compiler for AVR (ATmel
microcontrollers) on Cygwin (Windows). With earlier GCC version 4.1.2 always
smaller code was generated for one of the projects I'm working on than with
more recent versions of GCC. I tested with GCC 4.2.x a while ago and more
recently with GCC 4.3.3 (snapshot) and GCC 4.4 (snapshot also).

Since I'm mostly adding patches to GCC from the WinAVR project, I prepared a
freshly compiled GCC 4.3.3 (20081219) without these patches for this bug report
over here. The following is the version output of the compiler for my tests:

#> avr-gcc43 -v
Using built-in specs.
Target: avr
Configured with: ../gcc-4.3-20081218/configure --prefix=/usr/local --target=avr
--program-prefix=avr- --program-suffix=43 --enable-languages=c --disable-libssp
Thread model: single
gcc version 4.3.3 20081218 (prerelease) (GCC) 


Failure description:
GCC 4.3.3 does automatically inline (many) functions, when optimization setting
"-O2" is used. It even inlines static functions that are called several times
from all over the code within the same module. This heavily increases code
size.
Even worse, this code blowing automatic inline is also done with optimzation
setting "-Os" and therefore contradicts this optimization setting. With
GCC-4.1.2 this code increase because of (too much) automatic function inlining
could not be observed, although even GCC-4.1.2 does some automatic inlining.

Here's a stripped down code sample to demonstrate the bug:

/*
 * License: GPL
 * Copyright: (c) 2007 by Till Harbaum <till@harbaum.org>
 * Copyright: (c) 2008 Wolfgang Moser, http://d81.de
 */

/* #include <avr/wdt.h> */
/* some declarations from the include above for ATMega8 */
#define _SFR_IO8(io_addr) (*(volatile unsigned char *)((io_addr) + 0x20))
#define TOV0    0
#define TCNT0   _SFR_IO8(0x32)
#define TIFR    _SFR_IO8(0x38)
#define DDRC    _SFR_IO8(0x14)
#define DDRD    _SFR_IO8(0x11)
#define PORTC   _SFR_IO8(0x15)
#define PORTD   _SFR_IO8(0x12)

static void delay_wait_us( unsigned char timeout ) {
        __asm__ __volatile__ ("wdr");

        TCNT0 = timeout;
        TIFR |= (1 << (TOV0));

        /* wait until counter overflows */
        while(!(TIFR & (1 << (TOV0))));
}

static void delay_wait_us_ms( unsigned char timeout ) {
        delay_wait_us( timeout * 1000 );
}


void makeUseOfTimerWait( unsigned char val ) {
        delay_wait_us( 10 );
        DDRC |= 0x10;
        delay_wait_us( 10 );
        DDRD |= 0x20;

        delay_wait_us( 20 );
        PORTC &= ~0x10;
        delay_wait_us( 10 );
        PORTD &= ~0x20;

        delay_wait_us_ms( 5 );

        PORTC |= val & 0x10;
        delay_wait_us( 10 );
        PORTD |= val & 0x20;

        delay_wait_us( 10 );
}

Compiling this sample with: #> avr-gcc43 -save-temps -c -Wall -Os -mmcu=atmega8
autoinline.c
leads to the following module components: #> avr-nm -S --size-sort autoinline.o
00000000 0000009c T makeUseOfTimerWait

Both static functions, delay_wait_us() as well as delay_wait_ms() were inlined,
although the GCC manual clearly states that automatic function inlining would
only be enabled for optimization "-O3" of greater.

Normally, when optimizing for size with "-Os" (but also, when using "-O1" or
"-O2"), at least I would expect that at most functions are inlined that do not
increase code size. For the example above and setting "-Os", function
delay_wait_ms() may be inlined, because it is used only once. But function
delay_wait_us() must not be inlined. This behavior could be provoked by setting
special GCC internal configuration parameters like e.g.:

#> avr-gcc43 -save-temps --param max-inline-insns-auto=4 -c -Wall -Os
-mmcu=atmega8 autoinline.c

#> avr-nm -S --size-sort autoinline.o                                          
             00000000 00000012 t delay_wait_us
00000012 0000003e T makeUseOfTimerWait

That way code size reduce to 0x12+0x3e=0x50 instead of 0x9c (only taking
function symbols into account).


With kind regards, Wolfgang Moser


-- 
           Summary: gcc version 4.2.x, 4.3.x and 4.4.0 are doing (too much)
                    automatic inlining with -O1, -O2 and -Os
           Product: gcc
           Version: 4.3.3
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: cbm at d81 dot de
 GCC build triplet: i386/cygwin
  GCC host triplet: i386/cygwin
GCC target triplet: avr


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

[Bug target/38629] target-specific parameters for inline heuristics not defined for AVR

[steven at gcc dot gnu dot org]

------- Comment #1 from steven at gcc dot gnu dot org  2008-12-26 15:38 -------
GCC inline heuristics are just that: heuristics. They are not optimal for all
targets but only for those targets that they have been tuned for.

For AVR, nobody ever tuned the heuristics, despite several suggestions (see
e.g. bug 30908).  In this case, PARAM_INLINE_CALL_COST and MOVE_RATIO should be
tuned for AVR.

Therefore, target enhancement request.


-- 

steven at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|major                       |enhancement
             Status|UNCONFIRMED                 |NEW
          Component|c                           |target
     Ever Confirmed|0                           |1
   GCC host triplet|i386/cygwin                 |
   Last reconfirmed|0000-00-00 00:00:00         |2008-12-26 15:38:49
               date|                            |
            Summary|gcc version 4.2.x, 4.3.x and|target-specific parameters
                   |4.4.0 are doing (too much)  |for inline heuristics not
                   |automatic inlining with -O1,|defined for AVR
                   |-O2 and -Os                 |


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

[Bug target/38629] target-specific parameters for inline heuristics not defined for AVR

[forwards dot from dot bugzilla at d81 dot de]

------- Comment #2 from forwards dot from dot bugzilla at d81 dot de  2008-12-26 21:29 -------
Steven, thanks for your explanation. My point of view was that gcc-4.3.3 did do
inlining code for "-Os", "-O1", "-O2" although the manual clearly states that
it would not do inlining for these optimization levels.

     "-O2 Optimize even more. GCC performs... The compiler does not perform
          loop unrolling or function inlining when you specify -O2..."

But for "-O/-O1" it is mentioned:
     "-O turns on the following optimization flags:
      ... -finline-small-functions ..."

So the manual is a little ambiguous in this point. So to tune the heuristics
for the AVR target, one needs to tell the compiler:
 1) what's a "small function"
 2) tell about the costs
 3) maybe do something more

Actually, in GCC-4.4, gcc/config/avr/avr.c, somebody did already some tuning as
it seems to me:

avr_override_options (void)
{
  const struct mcu_type_s *t;

  flag_delete_null_pointer_checks = 0;

  if (!PARAM_SET_P (PARAM_INLINE_CALL_COST))
    set_param_value ("inline-call-cost", 5);
    ....

I don't feel qualified enough to find well working values for all AVR
architectures since I do know the Attiny2313, Mega8 and Mega644 a little bit.
Maybe someone is willing to help me.


Wolfgang


PS: Steven, do you have got administrative rights to edit my initial post.
Could you please convert Till Harbaum's eMail address into something that
cannot be parsed by eMail harvesting robots ('@' -> "(at)").


-- 

forwards dot from dot bugzilla at d81 dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |forwards dot from dot
                   |                            |bugzilla at d81 dot de


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

[Bug target/38629] target-specific parameters for inline heuristics not defined for AVR

[pinskia at gcc dot gnu dot org]

------- Comment #3 from pinskia at gcc dot gnu dot org  2008-12-26 21:35 -------
>    set_param_value ("inline-call-cost", 5);

This might be too high, the default for -O2/-O3 is 12.  That might be the real
issue.

Also -O2/-Os for 3.4 and above did the same inlining of static functions that
4.3 and above does.  Just it was not documented until 4.3 :).


-- 


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

[Bug target/38629] target-specific parameters for inline heuristics not defined for AVR

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2008-12-29 20:10 -------
There is a dup for this somewhere ... on pretty-ipa branch the call cost is 3.


-- 

rguenth at gcc dot gnu dot org changed:

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


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

[Bug target/38629] target-specific parameters for inline heuristics not defined for AVR

[federico at fsfe dot org]

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

--- Comment #8 from Federico Fissore <federico at fsfe dot org> ---
I forgot to say: this result came out of avr-gcc 4.8.1 (packaged by Arduino:
it's a 4.8.1 with two small patches applied [1]). It uses -Os optimization flag

[1] https://github.com/arduino/toolchain-avr/tree/master/gcc-patches

[Bug target/38629] target-specific parameters for inline heuristics not defined for AVR

[federico at fsfe dot org]

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

Federico Fissore <federico at fsfe dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |federico at fsfe dot org

--- Comment #7 from Federico Fissore <federico at fsfe dot org> ---
Created attachment 35278
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=35278&action=edit
big function gets inlined making a too big arduino sketch

I would like to revive this issue but submitting another test case.

The attached sketch file has a "move3Servos" function that gets inlined,
causing the sketch size to exceeds the size of the flash (49K with a max size
of 32K)

You can work around the problem adding an attribute noinline
__attribute__((noinline)) 
void move3Servos(Servo servo1Name, float target_pos1, Servo servo2Name, float
target_pos2, Servo servo3Name, float target_pos3, float easing) {

Doing so reduces the compiled sketch size down to 12K, which is a lot less than
49K

If you're interested in the original discussion, see
https://github.com/arduino/Arduino/issues/2930

[Bug target/38629] target-specific parameters for inline heuristics not defined for AVR

[hubicka at gcc dot gnu.org]

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

--- Comment #6 from Jan Hubicka <hubicka at gcc dot gnu.org> 2010-11-10 20:52:18 UTC ---
OK, at -Os the issue is that function is called once so inlining is a win.
Making multiple copies of it leads to GCC making clone:
delay_wait_us_ms.constprop.0:
.LFB3:  
        movl    $136, %edi
        jmp     delay_wait_us
.LFE3:  
and then calling it
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
        call    delay_wait_us_ms.constprop.0
at -Os,that is

With -O2 it is different story, we end up inlining everything. We get:
Analyzing function body size: delay_wait_us
  freq:  1000 size:  1 time:  1 __asm__ __volatile__("wdr");
  freq:  1000 size:  1 time:  1 MEM[(volatile unsigned char *)82B] ={v}
timeout_2(D);
  freq:  1000 size:  1 time:  1 D.2719_5 ={v} MEM[(volatile unsigned char
*)88B];
  freq:  1000 size:  1 time:  1 D.2720_6 = D.2719_5 | 1;
  freq:  1000 size:  1 time:  1 MEM[(volatile unsigned char *)88B] ={v}
D.2720_6;
  freq: 11111 size:  1 time:  1 D.2721_8 ={v} MEM[(volatile unsigned char
*)88B];
  freq: 11111 size:  0 time:  0 D.2722_9 = (int) D.2721_8;
  freq: 11111 size:  1 time:  1 D.2723_10 = D.2722_9 & 1;
  freq: 11111 size:  2 time:  2 if (D.2723_10 == 0)
  freq:  1000 size:  1 time:  2 return;
    Likely eliminated
Overall function body time: 51-2 size: 10-1
With function call overhead time: 51-13 size: 10-3

that fits in early-inlining-insns. With --param early-inlining-insns=0 we get
it right.  GCC inliner is guessing here that inlining such a small leaf
function will result in enough optimization so it pays back. I am not sure what
we can do here, early-inlining-insns is being pushed up by C++ code...

It is not terribly bad tradeoff even at -O2. I will try to get some data how
much early inlining insns cost us at -O2 and if it is too much, I will disable
the allowed growth for functions not declared inline.

[Bug target/38629] target-specific parameters for inline heuristics not defined for AVR

[hubicka at gcc dot gnu.org]

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

--- Comment #5 from Jan Hubicka <hubicka at gcc dot gnu.org> 2010-11-10 20:19:24 UTC ---
Hi,
I have reservations for making inline heuristics too target specific as it
would increase the testing matrix of inliner even more.  It is difficult to
satisfy everyone.  

We no longer inline delay_wait_us on i386, ARM should behave the same.  For
whatever reason we still inline delay_wait_us_ms.  I am checking why. Size is
estimated to grow by one instruction (that is bit underestimated) but with
--param early-inlining-insns=0 we should not inline.

Honza