public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Sebor <msebor@gmail.com>
To: Arkadiusz Drabczyk <arkadiusz@drabczyk.org>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c/67925 - update documentation on `inline'
Date: Thu, 15 Oct 2015 00:18:00 -0000	[thread overview]
Message-ID: <561EF0BB.7090808@gmail.com> (raw)
In-Reply-To: <20151014214245.GB20411@comp.lan>

On 10/14/2015 03:42 PM, Arkadiusz Drabczyk wrote:
> On Wed, Oct 14, 2015 at 08:36:43AM -0600, Martin Sebor wrote:
>> On 10/13/2015 04:47 PM, Arkadiusz Drabczyk wrote:
>>> * gcc/doc/extend.texi: documentation says that functions declared
>>> `inline' would not be integrated if they are called before they are
>>> defined or if they are recursive. Both of these statements is now
>>> false as shown in examples on Bugzilla.
>>
>> It might also be worth updating the note in the subsequent
>> paragraph and removing the mention of variable-length data types
>> which no longer prevent inlining.
>
> Done.  I also removed the mention of nested functions as the following
> code compiled with GCC 6.0 doesn't give any warning with -O2 -Winline
> and main() is the only function defined in assembler code:

I think this is the Ada-specific warning I mentioned (see
check_inlining_for_nested_subprog in gcc/ada/gcc-interface/trans.c)
so the part about nested functions needs to stay.

>> 	      = G_("function %q+F can never be inlined because "
>> 		   "it uses non-local goto");
>
> I tested of all of these and listed them in the documentation but
> wasn't able to reproduce this one.  The following code does not give
> any warning with -O2 -Winline:

The warning above is issued for non-local and computed goto. You can
find examples of both in the test suite (find gcc/testsuite/ -name
"*goto*.[cC]")

>
> #include <stdio.h>
> #include <stdlib.h>
> #include <setjmp.h>
>
> static jmp_buf buf;
>
> inline static void longjmp_test(int n)
> {
> 	puts("hi");
>
> 	if (n == 2)
> 		longjmp(buf, 2);
> }
>
> int main(int argc, char *argv[])
> {
> 	printf("%d\n", setjmp(buf));
> 	longjmp_test(atoi(argv[1]));
> 	puts("next line in a normal execution");
> 	exit(0);
> }
>
> Is this test case correct?

Not really. You can see the warning by calling setjmp in longjmp_test.

>  However, I get the following warning using
> __builtin_longjmp() instead of longjmp():
>
> warning: function 'longjmp_test' can never be inlined because it uses
> setjmp-longjmp exception handling [-Winline]

Right. You can see why in the comment in gcc/tree-inline.c:
             /* We can't inline functions that call __builtin_longjmp at
                all.  The non-local goto machinery really requires the
                destination be in a different function.  If we allow the
                function calling __builtin_longjmp to be inlined into the
                function calling __builtin_setjmp, Things will Go Awry.  */
             inline_forbidden_reason
               = G_("function %q+F can never be inlined because "
                    "it uses setjmp-longjmp exception handling");

>> 8------------------------------------------------------8<
> * gcc/doc/extend.texi: documentation says that functions declared
> `inline' would not be integrated if they are called before they are
> defined, if they are recursive, if they use variable-length data types
> or if they are nested.  All of these statements are now false and have
> been removed. Mention of setjmp(), __builtin_return() and
> __builtin_apply_args() has been added.

Great, thank you for taking the time to make these additional updates.
Modulo the nested functions, the patch looks good to me (someone else
will need to formally approve the final version).

Martin

> ---
>   gcc/doc/extend.texi | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 79440d3..be95cc3 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -7088,21 +7088,18 @@ function are integrated into the caller, and the function's address is
>   never used, then the function's own assembler code is never referenced.
>   In this case, GCC does not actually output assembler code for the
>   function, unless you specify the option @option{-fkeep-inline-functions}.
> -Some calls cannot be integrated for various reasons (in particular,
> -calls that precede the function's definition cannot be integrated, and
> -neither can recursive calls within the definition).  If there is a
> -nonintegrated call, then the function is compiled to assembler code as
> -usual.  The function must also be compiled as usual if the program
> -refers to its address, because that can't be inlined.
> +If there is a nonintegrated call, then the function is compiled to
> +assembler code as usual.  The function must also be compiled as usual if
> +the program refers to its address, because that can't be inlined.
>
>   @opindex Winline
>   Note that certain usages in a function definition can make it unsuitable
> -for inline substitution.  Among these usages are: variadic functions, use of
> -@code{alloca}, use of variable-length data types (@pxref{Variable Length}),
> -use of computed goto (@pxref{Labels as Values}), use of nonlocal goto,
> -and nested functions (@pxref{Nested Functions}).  Using @option{-Winline}
> -warns when a function marked @code{inline} could not be substituted,
> -and gives the reason for the failure.
> +for inline substitution.  Among these usages are: variadic functions,
> +use of @code{alloca}, use of computed goto (@pxref{Labels as Values}),
> +use of @code{setjmp} and use of @code{__builtin_return} or
> +@code{__builtin_apply_args}.  Using @option{-Winline} warns when a
> +function marked @code{inline} could not be substituted, and gives the
> +reason for the failure.
>
>   @cindex automatic @code{inline} for C++ member fns
>   @cindex @code{inline} automatic for C++ member fns
>

  reply	other threads:[~2015-10-15  0:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-13 22:47 Arkadiusz Drabczyk
2015-10-14 14:36 ` Martin Sebor
2015-10-14 21:42   ` Arkadiusz Drabczyk
2015-10-15  0:18     ` Martin Sebor [this message]
2015-10-15 12:24       ` Arkadiusz Drabczyk
2015-10-20  5:55         ` Jeff Law

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=561EF0BB.7090808@gmail.com \
    --to=msebor@gmail.com \
    --cc=arkadiusz@drabczyk.org \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).