| Summary: | [3.3/3.4] --param min-inline-insns not honoured | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Richard Biener <rguenth> |
| Component: | rtl-optimization | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | critical | CC: | gcc-bugs |
| Priority: | P2 | Keywords: | missed-optimization |
| Version: | 3.4.0 | ||
| Target Milestone: | 3.3.1 | ||
| 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 | ||
Fix: Apply the attached patch. Confirmed. Richard, I didn't follow the thread on gcc-patches completely. Is this now superseded by __attribute__ leafify? Thanks. Dara 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. 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... 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. 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. 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/ 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. 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.
Since it sounds like you have a testcase, a POOMA based one, can you attach it here? 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/ Subject: Bug 10679 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_3-branch Changes by: mmitchel@gcc.gnu.org 2003-07-23 07:13:42 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&only_with_tag=gcc-3_3-branch&r1=1.16114.2.682&r2=1.16114.2.683 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/tree-inline.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.38.2.10&r2=1.38.2.11 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=1.2261.2.240&r2=1.2261.2.241 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/opt/inline4.C.diff?cvsroot=gcc&only_with_tag=gcc-3_3-branch&r1=NONE&r2=1.1.2.1 Fixed in GCC 3.3.1 -- but not yet fixed on the mainline. 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 Fixed in GCC 3.4. |
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.