Re: -Winline option

[David Miller <davem at davemloft dot net>]

From: Florian Weimer <fweimer@redhat.com>
Date: Thu, 09 Apr 2015 17:09:54 +0200

> On 04/09/2015 04:17 PM, Stefan Liebler wrote:
> 
>>> I have a patch that adds a cast to size_t, which suppresses the warning
>>> on 32-bit platforms.
>> Yes, "(size_t) (1ULL << 32)" suppresses the warning (tested with gcc
>> 4.6.4, 4.7.4, 4.8.4, 4.9.2).
> 
> Okay, I will commit that shortly.
> 
>>> Using 1ULL << 31 would invalidate the test.
>>>
>>> Can you make a change to the test, so that it compiles, and check if you
>>> get the same inlining failure as Dave?
>>>
>> I get the following inlining failures with gcc 4.6.4/4.7.4 on 32/64bit:
>> In file included from tst-scratch_buffer.c:19:0:
>> tst-scratch_buffer.c: In function ʽdo_testʼ:
>> ../include/scratch_buffer.h:85:1: error: inlining failed in call to
>> ʽscratch_buffer_freeʼ: call is unlikely and code size would grow
>> [-Werror=inline]
>> tst-scratch_buffer.c:79:25: error: called from here [-Werror=inline]
> 
> If the line numbers are correct, that's the start of do_test:
> 
>     struct scratch_buffer buf;
>     scratch_buffer_init (&buf);
>     memset (buf.data, ' ', buf.length);
>     scratch_buffer_free (&buf);
> 
> And I don't see why this code path could be assumed to be unlikely.  It
> might be a GCC bug.  I don't see it with later versions.

These are the same warnings I get on sparc, FWIW.

> But the fact is that we want to use inline functions on unlikely paths,
> and there's nothing wrong with not inlining them there.  I don't know
> why we activate -Winline.  The option was apparently added to glibc when
> GCC did not support the always_inline attribute.  If we need inlining
> for correctness, we should just define the function with
> __always_inline.  This way, inlining failures will receive a warning
> under -Wattributes, which is enabled by default.  We can then drop
> -Winline and let GCC do what it thinks is best.
> 
> Comments?

I agree that -Winline should be removed.