Re: [PATCH v2] Add no_tail_call attribute

[Jeff Law <law at redhat dot com>]

On 07/03/2017 12:08 PM, Yuri Gribov wrote:
>>> 0001-Added-no_tail_call-attribute.patch
>>>
>>>
>>> From 1f4590e7a633c6335512b012578bddba7602b3c9 Mon Sep 17 00:00:00 2001
>>> From: Yury Gribov <tetra2005@gmail.com>
>>> Date: Sun, 28 May 2017 21:02:20 +0100
>>> Subject: [PATCH] Added no_tail_call attribute.
>>>
>>> gcc/
>>> 2017-05-29  Yury Gribov  <tetra2005@gmail.com>
>>>
>>>       * cgraphunit.c (cgraph_node::expand_thunk): Prevent
>>>       tailcalling functions marked with no_tail_call.
>>>       * gcc/doc/extend.texi: Document no_tail_call.
>>>       * tree-tailcall.c (find_tail_calls): Ditto.
>>>       * tree.c (comp_type_attributes): Treat no_tail_call
>>>       mismatch as error.
>>>
>>> gcc/c-family/
>>> 2017-05-29  Yury Gribov  <tetra2005@gmail.com>
>>>
>>>       * c-attribs.c: New attribute.
>>>
>>> gcc/testsuite/
>>> 2017-05-29  Yury Gribov  <tetra2005@gmail.com>
>>>
>>>       * gcc.dg/pr66826-1.c: New test.
>>>       * gcc.dg/pr66826-2.c: New test.
>> I think a "no_tail_call" attribute is quite reasonable -- more so than
>> some asm hack to prevent tail calling.
> 
> Thanks! Frankly I lost my hope on this...
Sorry that happened.  Sometimes things take a while, but we do try to
get to everything.


> 
>>> ---
>>>  gcc/c-family/c-attribs.c         |  1 +
>>>  gcc/cgraphunit.c                 |  6 ++++--
>>>  gcc/doc/extend.texi              |  7 +++++++
>>>  gcc/testsuite/gcc.dg/pr66826-1.c | 14 ++++++++++++++
>>>  gcc/testsuite/gcc.dg/pr66826-2.c |  6 ++++++
>>>  gcc/tree-tailcall.c              |  4 ++++
>>>  gcc/tree.c                       |  7 ++++---
>>>  7 files changed, 40 insertions(+), 5 deletions(-)
>>>  create mode 100644 gcc/testsuite/gcc.dg/pr66826-1.c
>>>  create mode 100644 gcc/testsuite/gcc.dg/pr66826-2.c
>>>
>>> diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
>>> index 695c58c..482db00 100644
>>> --- a/gcc/c-family/c-attribs.c
>>> +++ b/gcc/c-family/c-attribs.c
>>> @@ -345,6 +345,7 @@ const struct attribute_spec c_common_attribute_table[] =
>>>                             handle_bnd_instrument, false },
>>>    { "fallthrough",         0, 0, false, false, false,
>>>                             handle_fallthrough_attribute, false },
>>> +  { "no_tail_call",           0, 0, false, true, true, NULL, true },
>> Is no_tail_call supposed to be attached to the function's decl or type?
>>
>> ISTM this is most similar to noclone, noinline, no_icf and friends which
>> seem to attach the attribute to the decl rather than to the type.
> 
> Glibc people were worried that attribute would be lost when taking a
> pointer to function
> (https://sourceware.org/ml/libc-alpha/2017-01/msg00482.html). I think
> their reasoning was that return address is a shadow argument for
> dlsym-like functions so this would cause a (most likely inadvertent)
> ABI error.
Fair enough, but does the right thing happen if the function's
definition is decorated?  Can you add a test for that in the testsuite?

Assuming it works with the definition decorated, then this is OK for the
trunk.

jeff