Re: [PATCH] D12922: Add support for function attribute "notail"

This revision was automatically updated to reflect the committed changes. Closed by commit rL252369: Add support for function attribute 'not_tail_called'. (authored by ahatanak). Changed prior to commit: http://reviews.llvm.org/D12922?vs=39003=39607#toc Repository: rL LLVM

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak added a comment. Thanks, I'll see how the review for the llvm-side patch goes and commit both patches after it is approved. http://reviews.llvm.org/D12922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: [PATCH] D12922: Add support for function attribute "notail"

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/Sema/SemaDecl.cpp:5374 @@ +5373,3 @@ + + // Virtual functions cannot be marked as 'notail'. + if (auto *Attr = ND.getAttr())

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak added inline comments. Comment at: lib/Sema/SemaDecl.cpp:5374 @@ +5373,3 @@ + + // Virtual functions cannot be marked as 'notail'. + if (auto *Attr = ND.getAttr()) aaron.ballman wrote: > I am not home yet, and so I don't have the source code to try

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak updated this revision to Diff 38967. ahatanak added a comment. Fixed test cases: - Added a test case for the attribute being specified on a variable. - Added a missing 's' to "virtual function" in the check strings. http://reviews.llvm.org/D12922 Files: include/clang/Basic/Attr.td

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak added inline comments. Comment at: test/Sema/attr-notail.c:9 @@ +8,3 @@ +} + +int g0 __attribute__((not_tail_called)); // expected-warning {{'not_tail_called' attribute only applies to functions}} I didn't understand what kind of test case is required

Re: [PATCH] D12922: Add support for function attribute "notail"

aaron.ballman added inline comments. Comment at: test/Sema/attr-notail.c:9 @@ +8,3 @@ +} + +int g0 __attribute__((not_tail_called)); // expected-warning {{'not_tail_called' attribute only applies to functions}} ahatanak wrote: > I didn't understand what kind of

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak updated this revision to Diff 39003. ahatanak added a comment. Added test case that produces an error diagnostic if an argument is passed for not_tail_called. http://reviews.llvm.org/D12922 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td

Re: [PATCH] D12922: Add support for function attribute "notail"

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDecl.cpp:5374 @@ +5373,3 @@ + + // Virtual functions cannot be marked as 'notail'. + if (auto *Attr = ND.getAttr()) I am not home yet, and so I don't have the source code to try this out, but I have

Re: [PATCH] D12922: Add support for function attribute "notail"

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDecl.cpp:5374 @@ +5373,3 @@ + + // Virtual functions cannot be marked as 'notail'. + if (auto *Attr = ND.getAttr()) Is there a reason this is here instead of SemaDeclAttr.cpp? It seems like the decl

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak added inline comments. Comment at: lib/Sema/SemaDecl.cpp:5374 @@ +5373,3 @@ + + // Virtual functions cannot be marked as 'notail'. + if (auto *Attr = ND.getAttr()) aaron.ballman wrote: > Is there a reason this is here instead of SemaDeclAttr.cpp? It

Re: [PATCH] D12922: Add support for function attribute "notail"

ping On Mon, Oct 19, 2015 at 5:59 PM, Akira Hatanaka wrote: > ahatanak updated this revision to Diff 37816. > ahatanak added a comment. > > Address review comments: > > 1. Renamed the attribute to "not_tail_called". > > I chose "not_tail_called" over "notailcall" or "notail"

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak updated this revision to Diff 37816. ahatanak added a comment. Address review comments: 1. Renamed the attribute to "not_tail_called". I chose "not_tail_called" over "notailcall" or "notail" to better distinguish it from the attribute that is proposed in http://reviews.llvm.org/D12547

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak updated this revision to Diff 37437. ahatanak added a comment. Address some of the review comments: - Added a c++ test which tests the c++ spelling of the attribute on member functions and shows which virtual functions calls get marked as "notail" in the IR. - Added a check that was

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak marked an inline comment as done. Comment at: include/clang/Basic/AttrDocs.td:1619 @@ +1618,3 @@ + let Content = [{ +Tail call optimization is not performed on direct calls to a function marked ``notail``. + }]; Perhaps direct call isn't the right

Re: [PATCH] D12922: Add support for function attribute "notail"

> On 2015-Oct-13, at 09:55, Aaron Ballman wrote: > > aaron.ballman added a comment. > > In http://reviews.llvm.org/D12922#265452, @ahatanak wrote: > >> I think there are a couple of things that have to be discussed: >> >> 1. Name of the attribute: Should it be

Re: [PATCH] D12922: Add support for function attribute "notail"

aaron.ballman added a comment. In http://reviews.llvm.org/D12922#265452, @ahatanak wrote: > I think there are a couple of things that have to be discussed: > > 1. Name of the attribute: Should it be "notail" or "notailcall"? Perhaps it > should be named something like "nodirecttail" to indicate

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak updated this revision to Diff 37172. ahatanak added a comment. I've made some changes following the discussion I had and the feedback I got on the llvm-side patch. This is the link to the discussion thread: http://thread.gmane.org/gmane.comp.compilers.llvm.cvs/271105/ The difference

Re: [PATCH] D12922: Add support for function attribute "notail"

I wonder if 'notailcall' be better, since it contains a verb (like 'noinline'). I don't have a strong opinion; just putting the alternative out there. > On 2015-Sep-16, at 19:21, Akira Hatanaka via cfe-commits > wrote: > > ahatanak created this revision. >

Re: [PATCH] D12922: Add support for function attribute "notail"

dexonsmith added a subscriber: dexonsmith. dexonsmith added a comment. I wonder if 'notailcall' be better, since it contains a verb (like 'noinline'). I don't have a strong opinion; just putting the alternative out there. http://reviews.llvm.org/D12922

Re: [PATCH] D12922: Add support for function attribute "notail"

ahatanak added a comment. The llvm-side patch is here: http://reviews.llvm.org/D12923 http://reviews.llvm.org/D12922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D12922: Add support for function attribute "notail"

ahatanak created this revision. ahatanak added a reviewer: aaron.ballman. ahatanak added a subscriber: cfe-commits. This patch adds support for a new function attribute "notail". The attribute is used to disable tail call optimization on calls to functions marked with the attribute. This is