User account creation filtered due to spam.

Bug 10679 - [3.3/3.4] --param min-inline-insns not honoured
Summary: [3.3/3.4] --param min-inline-insns not honoured
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 3.4.0
: P2 critical
Target Milestone: 3.3.1
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2003-05-08 12:16 UTC by Richard Biener
Modified: 2004-01-17 04:22 UTC (History)
1 user (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-linux-pc-gnu
Build: i686-pc-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2003-06-04 19:29:56


Attachments
tree-inline-patch (570 bytes, patch)
2003-05-21 15:17 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2003-05-08 12:16:00 UTC
If specifying --param min-inline-insns one expects from the manual all functions with size <= min-inline-insns to be inlined. This is not the case if inlining repeatedly into a large function as can be seen from comments/code in tree-inline.c:inlinable_function_p (line 1003-)

This severly pessimizes scientific code based on the POOMA library, where inlining of small function is vitally important. After applying the attached patch, >30% performance can be gained and the --param min-inline-insns behaves as documented.

No testcase, the defect can be pinpointed in the gcc source. A testcase would look like

inline void bar(void) { }
void foo(void)
{
    bar(); bar(); bar(); /* repeat 10000 times... */
}

at a certain point, bar()s would be no longer inlined and
the resulting foo() would not be empty.

Release:
g++-3.3 (GCC) 3.3 20030505 (prerelease), g++-3.4 (GCC) 3.4 20030505 (experimental)

Environment:
ia32 gnu-linux

How-To-Repeat:
Repeatedly inline a small function into a large one.
Comment 1 Richard Biener 2003-05-08 12:16:00 UTC
Fix:
Apply the attached patch.
Comment 2 Dara Hazeghi 2003-06-04 19:29:55 UTC
Confirmed. Richard, I didn't follow the thread on gcc-patches completely. Is this now superseded 
by __attribute__ leafify? Thanks.

Dara
Comment 3 Richard Biener 2003-06-04 20:44:55 UTC
Subject: Re:  [3.3/3.4] --param min-inline-insns not
 honoured

On 4 Jun 2003, dhazeghi@yahoo.com wrote:

> PLEASE REPLY TO gcc-bugzilla@gcc.gnu.org ONLY, *NOT* gcc-bugs@gcc.gnu.org.
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10679
>
>
> dhazeghi@yahoo.com changed:
>
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>              Status|UNCONFIRMED                 |NEW
>      Ever Confirmed|                            |1
>   GCC build triplet|                            |i686-pc-linux-gnu
>    GCC host triplet|                            |i686-pc-linux-gnu
>  GCC target triplet|                            |i686-linux-pc-gnu
>    Last reconfirmed|0000-00-00 00:00:00         |2003-06-04 19:29:56
>                date|                            |
>
>
> ------- Additional Comments From dhazeghi@yahoo.com  2003-06-04 19:29 -------
> Confirmed. Richard, I didn't follow the thread on gcc-patches completely. Is this now superseded
> by __attribute__ leafify? Thanks.

No. __attribute__((leafify)) is additional and also not accepted, nor
commented on in concept. (Though the actual patch I submitted
accidentially removes the offending check of this PR).

Richard.

Comment 4 Dara Hazeghi 2003-06-04 21:44:45 UTC
Darn! I should have read the thread more carefully, sorry for the noise... Do you think it would be 
worthwhile to submit a generic PR stating that the inliner is broken, period, and set it to P1, 
blocker for 3.4? It seems rather ridiculous to be planning yet another release without this IMHO 
extremely serious problem being solved. I really don't think waiting for tree-ssa is a valid option...
Comment 5 Richard Biener 2003-06-04 21:54:55 UTC
Subject: Re:  [3.3/3.4] --param min-inline-insns not
 honoured

On 4 Jun 2003, dhazeghi@yahoo.com wrote:

> PLEASE REPLY TO gcc-bugzilla@gcc.gnu.org ONLY, *NOT* gcc-bugs@gcc.gnu.org.
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10679
>
>
>
> ------- Additional Comments From dhazeghi@yahoo.com  2003-06-04 21:44 -------
> Darn! I should have read the thread more carefully, sorry for the noise... Do you think it would be
> worthwhile to submit a generic PR stating that the inliner is broken, period, and set it to P1,
> blocker for 3.4? It seems rather ridiculous to be planning yet another release without this IMHO
> extremely serious problem being solved. I really don't think waiting for tree-ssa is a valid option...

Yes, but everyone seems to set their bets on tree-ssa to make inlining
"trivially" work. But with this PR fixed, and __attribute__((leafify)) I'm
currently happy (apart from far from ideal CSE/GCSE/loop on the resulting
large functions).

Richard.

Comment 6 Andrew Pinski 2003-07-15 18:58:39 UTC
 think this is fixed in 3.4 (20030715) with the new inlining counting mechanism so that 
since bar is empty, gcc will inline it always. I just tried one with 30000 function calls to bar 
and it always inlined bar into foo at -O2.  It took over 90 seconds though it is a little too 
much time spent on it though.
Comment 7 Richard Biener 2003-07-16 07:25:42 UTC
Subject: Re:  [3.3/3.4] --param min-inline-insns not
 honoured

On 15 Jul 2003, pinskia at physics dot uc dot edu wrote:

>  think this is fixed in 3.4 (20030715) with the new inlining counting mechanism so that
> since bar is empty, gcc will inline it always. I just tried one with 30000 function calls to bar
> and it always inlined bar into foo at -O2.  It took over 90 seconds though it is a little too
> much time spent on it though.

Are you sure this is true in all circumstances? In gcc/tree-inline.c I
still see the offending check at line 1020. So if not inlinable_function_p
is called with nolimit set always, we'll still hit this bug (I suppose
this happens at least with -fno-unit-at-a-time). If it is, this function
could be cleaned up a lot.

Richard.

--
Richard Guenther <richard dot guenther at uni-tuebingen dot de>
WWW: http://www.tat.physik.uni-tuebingen.de/~rguenth/

Comment 8 Andrew Pinski 2003-07-18 14:35:35 UTC
The check is still there but if the function that gcc is inlining is empty (which only happens with the 
new inlining counting mechanism), gcc will now always inline that function so that part is fixed, if 
on the other hand the function is not empty, you can and will hit that limit which you pointed out.
Comment 9 Richard Biener 2003-07-19 15:02:37 UTC
Subject: Re:  [3.3/3.4] --param min-inline-insns not
 honoured

On 18 Jul 2003, pinskia at physics dot uc dot edu wrote:

> The check is still there but if the function that gcc is inlining is empty (which only happens with the
> new inlining counting mechanism), gcc will now always inline that function so that part is fixed, if
> on the other hand the function is not empty, you can and will hit that limit which you pointed out.

Yes, constructing a testcase for this bug is not easy, but placing a
warning inside the check causes >10000 hits on a medium size POOMA based
application. So this bug really triggers and hurts both runtime and
codesize.

Richard.

Comment 10 Andrew Pinski 2003-07-21 02:05:40 UTC
Since it sounds like you have a testcase, a POOMA based one, can you attach it here? 
Comment 11 Richard Biener 2003-07-21 09:39:15 UTC
Subject: Re:  [3.3/3.4] --param min-inline-insns not
 honoured

On 21 Jul 2003, pinskia at physics dot uc dot edu wrote:

> Since it sounds like you have a testcase, a POOMA based one, can you attach it here?

I tried to verify my POOMA testcase with todays mainline and were not able
to reproduce the problems I have seen (and still see with 3.3.1). So the
new counting seems to be a huge improvement, though I still think the
check in tree-inline.c should be removed. Consider a not empty function
like

void foo(int i)
{
   switch (i) {
   case 1:
     bar();
     break;
   default:;
   }
}

If you have lots of calls to foo(x) with x!=1 you're going to hit this
limit even though the optimizer should be able to optimize away the
inlined function.

At the very last, if the check is not removed, the documentation needs to
be updated to honour it.

Richard.

--
Richard Guenther <richard dot guenther at uni-tuebingen dot de>
WWW: http://www.tat.physik.uni-tuebingen.de/~rguenth/

Comment 13 Mark Mitchell 2003-07-23 07:43:54 UTC
Fixed in GCC 3.3.1 -- but not yet fixed on the mainline.
Comment 14 CVS Commits 2003-07-23 16:45:20 UTC
Subject: Bug 10679

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	mmitchel@gcc.gnu.org	2003-07-23 16:45:15

Modified files:
	gcc            : ChangeLog tree-inline.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/opt: inline4.C 

Log message:
	PR optimization/10679
	* tree-inline.c (inlinable_function_p): Honor MIN_INLINE_INSNS.
	
	PR optimization/10679
	* g++.dg/opt/inline4.C: New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.601&r2=2.602
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-inline.c.diff?cvsroot=gcc&r1=1.68&r2=1.69
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.2910&r2=1.2911
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/opt/inline4.C.diff?cvsroot=gcc&r1=1.1&r2=1.2

Comment 15 Mark Mitchell 2003-07-23 16:49:14 UTC
Fixed in GCC 3.4.