This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
-Winline option (was: Re: [PATCH 06/25] Add struct scratch_buffer and its internal helper functions)
- From: Florian Weimer <fweimer at redhat dot com>
- To: libc-alpha at sourceware dot org
- Date: Thu, 09 Apr 2015 17:09:54 +0200
- Subject: -Winline option (was: Re: [PATCH 06/25] Add struct scratch_buffer and its internal helper functions)
- Authentication-results: sourceware.org; auth=none
- References: <cover dot 1425285061 dot git dot fweimer at redhat dot com> <7a6fe503fb764beee3d5b89662d3bbf65242161c dot 1425285061 dot git dot fweimer at redhat dot com> <54F4BB15 dot 7070409 at cs dot ucla dot edu> <550C37F7 dot 10504 at redhat dot com> <550C4AE0 dot 60205 at cs dot ucla dot edu> <55103BEA dot 7070305 at redhat dot com> <5510505B dot 8060209 at cs dot ucla dot edu> <alpine dot DEB dot 2 dot 10 dot 1503231839380 dot 14930 at digraph dot polyomino dot org dot uk> <55105F2C dot 6040400 at redhat dot com> <alpine dot DEB dot 2 dot 10 dot 1503231845410 dot 14930 at digraph dot polyomino dot org dot uk> <55105FED dot 80004 at redhat dot com> <551C1C9B dot 1060807 at redhat dot com> <551D8AE1 dot 5000805 at redhat dot com> <551DE8EA dot 6080200 at cs dot ucla dot edu> <5522BC03 dot 5090300 at redhat dot com> <5523007D dot 9080906 at cs dot ucla dot edu> <55239F13 dot 5040700 at redhat dot com> <mg0lfr$ovf$1 at ger dot gmane dot org> <5523EABF dot 9050003 at redhat dot com> <5523FCA6 dot 2080504 at redhat dot com> <mg5jma$tuo$1 at ger dot gmane dot org> <55265D8E dot 6080001 at redhat dot com> <mg61lf$fh0$1 at ger dot gmane dot org>
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.
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?
--
Florian Weimer / Red Hat Product Security