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

[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

[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

[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

[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

[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

[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

[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

[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