| Message ID | 874m2y8qt5.fsf@atmel.com |
|---|---|
| State | New |
| Headers | show |
On 11/23/2016 02:54 AM, Senthil Kumar Selvaraj wrote: > Hi, > > The below patch fixes uninit-19.c for avr by adding > -finline-small-functions for avr. > > The test fails for avr because fn1 does not get inlined into fn2. Inlining > occurs for x86_64 because fn1's computed size equals call_stmt_size. For > avr, 32 bit memory moves are more expensive, and b[3] = p10[a] results in > a bigger size for fn1, resulting in estimate_growth > 0 and no inlining. > > Adding -finline-small-functions forces early inliner to inline fn1, > making the test pass. > > Committed to trunk. > > Regards > Senthil > > gcc/testsuite/ChangeLog > > 2016-11-23 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > * gcc.dg/uninit-19.c: Add -finline-small-functions for avr. How about instead forcing inlining via the always_inline attribute? If inlining is indeed expected and necessary, that seems better than -finline-small-functions. Jeff
Jeff Law writes: > On 11/23/2016 02:54 AM, Senthil Kumar Selvaraj wrote: >> Hi, >> >> The below patch fixes uninit-19.c for avr by adding >> -finline-small-functions for avr. >> >> The test fails for avr because fn1 does not get inlined into fn2. Inlining >> occurs for x86_64 because fn1's computed size equals call_stmt_size. For >> avr, 32 bit memory moves are more expensive, and b[3] = p10[a] results in >> a bigger size for fn1, resulting in estimate_growth > 0 and no inlining. >> >> Adding -finline-small-functions forces early inliner to inline fn1, >> making the test pass. >> >> Committed to trunk. >> >> Regards >> Senthil >> >> gcc/testsuite/ChangeLog >> >> 2016-11-23 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >> >> * gcc.dg/uninit-19.c: Add -finline-small-functions for avr. > How about instead forcing inlining via the always_inline attribute? If > inlining is indeed expected and necessary, that seems better than > -finline-small-functions. I considered it, but a previous discussion in the mailing list backed off from a similar proposal (https://gcc.gnu.org/ml/gcc-patches/2015-02/msg00517.html), and I wasn't sure if always_inline would break the pic/non-pic distinction somehow. Should I revert this and mark the function always_inline/static? > > Jeff
On 11/24/2016 12:24 AM, Senthil Kumar Selvaraj wrote: > > Jeff Law writes: > >> On 11/23/2016 02:54 AM, Senthil Kumar Selvaraj wrote: >>> Hi, >>> >>> The below patch fixes uninit-19.c for avr by adding >>> -finline-small-functions for avr. >>> >>> The test fails for avr because fn1 does not get inlined into fn2. Inlining >>> occurs for x86_64 because fn1's computed size equals call_stmt_size. For >>> avr, 32 bit memory moves are more expensive, and b[3] = p10[a] results in >>> a bigger size for fn1, resulting in estimate_growth > 0 and no inlining. >>> >>> Adding -finline-small-functions forces early inliner to inline fn1, >>> making the test pass. >>> >>> Committed to trunk. >>> >>> Regards >>> Senthil >>> >>> gcc/testsuite/ChangeLog >>> >>> 2016-11-23 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >>> >>> * gcc.dg/uninit-19.c: Add -finline-small-functions for avr. >> How about instead forcing inlining via the always_inline attribute? If >> inlining is indeed expected and necessary, that seems better than >> -finline-small-functions. > > I considered it, but a previous discussion in the mailing list backed > off from a similar proposal > (https://gcc.gnu.org/ml/gcc-patches/2015-02/msg00517.html), and I wasn't > sure if always_inline would break the pic/non-pic distinction somehow. > > Should I revert this and mark the function always_inline/static? It's often not obvious what the right thing to do is. I think it'll ultimately come down to trying to make an educated guess what the test is really trying to do and whether or not it it inherently depends on inlining. Your call whether to go back and use the always_inline attribute on that particular test. Jeff
Index: testsuite/gcc.dg/uninit-19.c =================================================================== --- testsuite/gcc.dg/uninit-19.c (revision 242741) +++ testsuite/gcc.dg/uninit-19.c (working copy) @@ -1,5 +1,6 @@ /* { dg-do compile } */ /* { dg-options "-O -Wuninitialized" } */ +/* { dg-additional-options "-finline-small-functions" { target avr-*-* } } */ int a, l, m; float *b; @@ -10,7 +11,7 @@ unsigned char *c2, float *p10) { if (p1 & 8) - b[3] = p10[a]; /* 13. */ + b[3] = p10[a]; /* 14. */ } void @@ -19,8 +20,8 @@ float *n; if (l & 6) n = &c + m; - fn1 (l, &d, &e, &g, &i, &h, &k, n); /* 22. */ + fn1 (l, &d, &e, &g, &i, &h, &k, n); /* 23. */ } -/* { dg-warning "may be used uninitialized" "" { target { { nonpic } || { hppa*64*-*-* } } } 13 } */ -/* { dg-warning "may be used uninitialized" "" { target { ! { { nonpic } || { hppa*64*-*-* } } } } 22 } */ +/* { dg-warning "may be used uninitialized" "" { target { { nonpic } || { hppa*64*-*-* } } } 14 } */ +/* { dg-warning "may be used uninitialized" "" { target { ! { { nonpic } || { hppa*64*-*-* } } } } 23 } */
Hi, The below patch fixes uninit-19.c for avr by adding -finline-small-functions for avr. The test fails for avr because fn1 does not get inlined into fn2. Inlining occurs for x86_64 because fn1's computed size equals call_stmt_size. For avr, 32 bit memory moves are more expensive, and b[3] = p10[a] results in a bigger size for fn1, resulting in estimate_growth > 0 and no inlining. Adding -finline-small-functions forces early inliner to inline fn1, making the test pass. Committed to trunk. Regards Senthil gcc/testsuite/ChangeLog 2016-11-23 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> * gcc.dg/uninit-19.c: Add -finline-small-functions for avr.