You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2014/05/28 03:41:01 UTC

warning: always_inline function might not be inlinable

Building latest trunk with gcc 4.7:

subversion/libsvn_fs_fs/index.c:351:1: warning: always_inline function might not be inlinable [-Wattributes]
subversion/libsvn_fs_x/index.c:356:1: warning: always_inline function might not be inlinable [-Wattributes]
subversion/libsvn_fs_x/index.c:339:1: warning: always_inline function might not be inlinable [-Wattributes]
subversion/libsvn_ra_svn/marshal.c:398:1: warning: always_inline function might not be inlinable [-Wattributes]

If the inlining is really required, we should fix the code such that it
either inlines the function or fails to compile, rather than this
halfway mode.

If inline is not required, we should demote the always_inline to a plain
APR_INLINE, which should sidestep the warning.

Re: warning: always_inline function might not be inlinable

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, May 28, 2014 at 4:15 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:

> Stefan Fuhrmann wrote on Wed, May 28, 2014 at 12:30:36 +0200:
> > On Wed, May 28, 2014 at 3:41 AM, Daniel Shahaf <d.s@daniel.shahaf.name
> >wrote:
> >
> > > Building latest trunk with gcc 4.7:
> > >
> > > subversion/libsvn_fs_fs/index.c:351:1: warning: always_inline function
> > > might not be inlinable [-Wattributes]
> > > subversion/libsvn_fs_x/index.c:356:1: warning: always_inline function
> > > might not be inlinable [-Wattributes]
> > > subversion/libsvn_fs_x/index.c:339:1: warning: always_inline function
> > > might not be inlinable [-Wattributes]
> > > subversion/libsvn_ra_svn/marshal.c:398:1: warning: always_inline
> function
> > > might not be inlinable [-Wattributes]
> > >
> > > If the inlining is really required, we should fix the code such that it
> > > either inlines the function or fails to compile, rather than this
> > > halfway mode.
> > >
> >
> > None of these inlines is required for functional correctness.
> > It is just that the compiler made a poor choice of those hot
> > code paths.
> >
>
> Could you please clarify this in the docstrings, as well?
>
> Right now they say:
>
>  * The forced inline is required as e.g. GCC would inline read() into here
>  * instead of lining the simple buffer access into callers of get().
>
> which could be mistaken as implying "required for functional
> correctness".
>
> I'd do this myself but you'd probably be able to describe what they
> _are_ required for better.
>

Done in r1598273.

-- Stefan^2.

Re: warning: always_inline function might not be inlinable

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Fuhrmann wrote on Wed, May 28, 2014 at 12:30:36 +0200:
> On Wed, May 28, 2014 at 3:41 AM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:
> 
> > Building latest trunk with gcc 4.7:
> >
> > subversion/libsvn_fs_fs/index.c:351:1: warning: always_inline function
> > might not be inlinable [-Wattributes]
> > subversion/libsvn_fs_x/index.c:356:1: warning: always_inline function
> > might not be inlinable [-Wattributes]
> > subversion/libsvn_fs_x/index.c:339:1: warning: always_inline function
> > might not be inlinable [-Wattributes]
> > subversion/libsvn_ra_svn/marshal.c:398:1: warning: always_inline function
> > might not be inlinable [-Wattributes]
> >
> > If the inlining is really required, we should fix the code such that it
> > either inlines the function or fails to compile, rather than this
> > halfway mode.
> >
> 
> None of these inlines is required for functional correctness.
> It is just that the compiler made a poor choice of those hot
> code paths.
> 

Could you please clarify this in the docstrings, as well?

Right now they say:

 * The forced inline is required as e.g. GCC would inline read() into here
 * instead of lining the simple buffer access into callers of get().

which could be mistaken as implying "required for functional
correctness".

I'd do this myself but you'd probably be able to describe what they
_are_ required for better.

> > If inline is not required, we should demote the always_inline to a plain
> > APR_INLINE, which should sidestep the warning.
> >
> 
> As it turned out, always_inline does not imply inline. So,
> the right fix here is to always add APR_INLINE (except for
> Visual C where __forceinline implies inline).
> 
> r1597962 should do the trick.
> 

Thanks!

> -- Stefan^2.

Re: warning: always_inline function might not be inlinable

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, May 28, 2014 at 3:41 AM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:

> Building latest trunk with gcc 4.7:
>
> subversion/libsvn_fs_fs/index.c:351:1: warning: always_inline function
> might not be inlinable [-Wattributes]
> subversion/libsvn_fs_x/index.c:356:1: warning: always_inline function
> might not be inlinable [-Wattributes]
> subversion/libsvn_fs_x/index.c:339:1: warning: always_inline function
> might not be inlinable [-Wattributes]
> subversion/libsvn_ra_svn/marshal.c:398:1: warning: always_inline function
> might not be inlinable [-Wattributes]
>
> If the inlining is really required, we should fix the code such that it
> either inlines the function or fails to compile, rather than this
> halfway mode.
>

None of these inlines is required for functional correctness.
It is just that the compiler made a poor choice of those hot
code paths.

If inline is not required, we should demote the always_inline to a plain
> APR_INLINE, which should sidestep the warning.
>

As it turned out, always_inline does not imply inline. So,
the right fix here is to always add APR_INLINE (except for
Visual C where __forceinline implies inline).

r1597962 should do the trick.

-- Stefan^2.