| Message ID | 20230223071525.89207-1-heinrich.schuchardt@canonical.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | lib: bzip2: remove inlining | expand |
On Thu, Feb 23, 2023 at 08:15:25AM +0100, Heinrich Schuchardt wrote: > Compiling sandbox_defconfig with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y and > gcc 12.2.0-14ubuntu1 leads to a build error: > > lib/bzip2/bzlib.c: In function 'BZ2_decompress': > lib/bzip2/bzlib.c:726:18: error: inlining failed in call to > 'always_inline' 'BZ2_indexIntoF': function not considered for inlining > 726 | __inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab ) > | ^ > > Leave it to the compiler if it inlines or not. > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> What did previous compilers do here? If we're telling the compiler to always inline, presumably for good reason, we shouldn't just stop.
On 2/23/23 17:30, Tom Rini wrote: > On Thu, Feb 23, 2023 at 08:15:25AM +0100, Heinrich Schuchardt wrote: >> Compiling sandbox_defconfig with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y and >> gcc 12.2.0-14ubuntu1 leads to a build error: >> >> lib/bzip2/bzlib.c: In function 'BZ2_decompress': >> lib/bzip2/bzlib.c:726:18: error: inlining failed in call to >> 'always_inline' 'BZ2_indexIntoF': function not considered for inlining >> 726 | __inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab ) >> | ^ >> >> Leave it to the compiler if it inlines or not. >> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > What did previous compilers do here? If we're telling the compiler to > always inline, presumably for good reason, we shouldn't just stop. > Inlining may make the code a bit faster. But without inlining it would be smaller. Grep for BZ_GET_SMALL to find where the inlined function is used. In test/compression.c we check the result. 'ut compression' does not find a problem. Best regards Heinrich
On Thu, Feb 23, 2023 at 05:39:08PM +0100, Heinrich Schuchardt wrote: > On 2/23/23 17:30, Tom Rini wrote: > > On Thu, Feb 23, 2023 at 08:15:25AM +0100, Heinrich Schuchardt wrote: > > > Compiling sandbox_defconfig with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y and > > > gcc 12.2.0-14ubuntu1 leads to a build error: > > > > > > lib/bzip2/bzlib.c: In function 'BZ2_decompress': > > > lib/bzip2/bzlib.c:726:18: error: inlining failed in call to > > > 'always_inline' 'BZ2_indexIntoF': function not considered for inlining > > > 726 | __inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab ) > > > | ^ > > > > > > Leave it to the compiler if it inlines or not. > > > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > > > What did previous compilers do here? If we're telling the compiler to > > always inline, presumably for good reason, we shouldn't just stop. > > > > Inlining may make the code a bit faster. But without inlining it would be > smaller. Grep for BZ_GET_SMALL to find where the inlined function is used. > > In test/compression.c we check the result. 'ut compression' does not find a > problem. OK, and I'm wondering if the compiler regressed.
On 2/23/23 17:40, Tom Rini wrote: > On Thu, Feb 23, 2023 at 05:39:08PM +0100, Heinrich Schuchardt wrote: >> On 2/23/23 17:30, Tom Rini wrote: >>> On Thu, Feb 23, 2023 at 08:15:25AM +0100, Heinrich Schuchardt wrote: >>>> Compiling sandbox_defconfig with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y and >>>> gcc 12.2.0-14ubuntu1 leads to a build error: >>>> >>>> lib/bzip2/bzlib.c: In function 'BZ2_decompress': >>>> lib/bzip2/bzlib.c:726:18: error: inlining failed in call to >>>> 'always_inline' 'BZ2_indexIntoF': function not considered for inlining >>>> 726 | __inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab ) >>>> | ^ >>>> >>>> Leave it to the compiler if it inlines or not. >>>> >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >>> >>> What did previous compilers do here? If we're telling the compiler to >>> always inline, presumably for good reason, we shouldn't just stop. >>> >> >> Inlining may make the code a bit faster. But without inlining it would be >> smaller. Grep for BZ_GET_SMALL to find where the inlined function is used. >> >> In test/compression.c we check the result. 'ut compression' does not find a >> problem. > > OK, and I'm wondering if the compiler regressed. > CONFIG_CC_OPTIMIZE_FOR_DEBUG is not used in any of our defconfigs and has been introduced long after bzip2. Typically CONFIG_CC_OPTIMIZE_FOR_DEBUG is used in conjunction with CONFIG_LTO=n. In this case the error does not occur. It seems that gcc with -Og -flto has an issue. Best regards Heinrich
On Thu, Feb 23, 2023 at 10:53:44PM +0100, Heinrich Schuchardt wrote: > > > On 2/23/23 17:40, Tom Rini wrote: > > On Thu, Feb 23, 2023 at 05:39:08PM +0100, Heinrich Schuchardt wrote: > > > On 2/23/23 17:30, Tom Rini wrote: > > > > On Thu, Feb 23, 2023 at 08:15:25AM +0100, Heinrich Schuchardt wrote: > > > > > Compiling sandbox_defconfig with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y and > > > > > gcc 12.2.0-14ubuntu1 leads to a build error: > > > > > > > > > > lib/bzip2/bzlib.c: In function 'BZ2_decompress': > > > > > lib/bzip2/bzlib.c:726:18: error: inlining failed in call to > > > > > 'always_inline' 'BZ2_indexIntoF': function not considered for inlining > > > > > 726 | __inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab ) > > > > > | ^ > > > > > > > > > > Leave it to the compiler if it inlines or not. > > > > > > > > > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > > > > > > > > What did previous compilers do here? If we're telling the compiler to > > > > always inline, presumably for good reason, we shouldn't just stop. > > > > > > > > > > Inlining may make the code a bit faster. But without inlining it would be > > > smaller. Grep for BZ_GET_SMALL to find where the inlined function is used. > > > > > > In test/compression.c we check the result. 'ut compression' does not find a > > > problem. > > > > OK, and I'm wondering if the compiler regressed. > > > > CONFIG_CC_OPTIMIZE_FOR_DEBUG is not used in any of our defconfigs and has > been introduced long after bzip2. > > Typically CONFIG_CC_OPTIMIZE_FOR_DEBUG is used in conjunction with > CONFIG_LTO=n. In this case the error does not occur. > > It seems that gcc with -Og -flto has an issue. OK, so this combination is fine with: gcc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0 Copyright (C) 2021 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. So I'd really like to get some compiler people to confirm that there's not a regression on their end here.
diff --git a/lib/bzip2/bzlib.c b/lib/bzip2/bzlib.c index bd589aa810..b28f14b540 100644 --- a/lib/bzip2/bzlib.c +++ b/lib/bzip2/bzlib.c @@ -723,7 +723,7 @@ void unRLE_obuf_to_output_FAST ( DState* s ) /*---------------------------------------------------*/ -__inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab ) +Int32 BZ2_indexIntoF(Int32 indx, Int32 *cftab) { Int32 nb, na, mid; nb = 0;
Compiling sandbox_defconfig with CONFIG_CC_OPTIMIZE_FOR_DEBUG=y and gcc 12.2.0-14ubuntu1 leads to a build error: lib/bzip2/bzlib.c: In function 'BZ2_decompress': lib/bzip2/bzlib.c:726:18: error: inlining failed in call to 'always_inline' 'BZ2_indexIntoF': function not considered for inlining 726 | __inline__ Int32 BZ2_indexIntoF ( Int32 indx, Int32 *cftab ) | ^ Leave it to the compiler if it inlines or not. Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> --- lib/bzip2/bzlib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)