Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-12 Thread Akira Hatanaka via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL252986: Add support for function attribute 'disable_tail_calls'. (authored by ahatanak). Changed prior to commit: http://reviews.llvm.org/D12547?vs=40060&id=40099#toc Repository: rL LLVM http://revi

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-12 Thread Akira Hatanaka via cfe-commits
ahatanak marked an inline comment as done. ahatanak added a comment. I'll commit this patch shortly. Thank you for the review. http://reviews.llvm.org/D12547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-12 Thread Aaron Ballman via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! http://reviews.llvm.org/D12547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-12 Thread Akira Hatanaka via cfe-commits
ahatanak updated this revision to Diff 40060. ahatanak marked 2 inline comments as done. ahatanak added a comment. Added "expected-no-diagnostics" to test case. http://reviews.llvm.org/D12547 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td lib/CodeGen/CGCall.cpp lib/S

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-12 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments. Comment at: test/SemaCXX/attr-disable-tail-calls.cpp:2 @@ +1,3 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s + +class B { Is this file missing an expected-no-diagnostics? http://reviews.llvm.org/D12547 ___

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-11 Thread Akira Hatanaka via cfe-commits
ahatanak updated this revision to Diff 40011. ahatanak added a comment. Address review comments. http://reviews.llvm.org/D12547 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td lib/CodeGen/CGCall.cpp lib/Sema/SemaDeclAttr.cpp test/CodeGen/attr-disable-tail-calls.c

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-11 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. In http://reviews.llvm.org/D12547#287153, @ahatanak wrote: > Marking virtual functions as disable_tail_calls is fine since > disable_tail_calls affects the call sites inside the body of the marked > function. In your example, it prevents tail call optimization on

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-11 Thread Akira Hatanaka via cfe-commits
ahatanak added a comment. Marking virtual functions as disable_tail_calls is fine since disable_tail_calls affects the call sites inside the body of the marked function. In your example, it prevents tail call optimization on call sites inside B::g, but doesn't affect call sites in D::g. http:

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-11 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. Missing tests demonstrating use of the C++ spelling of the attribute. Perhaps a test showing it on a member function would be useful. Out of curiosity, what would be the expected behavior of the following: struct B { int g(int); [[clang::disable_tail_cal

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-09 Thread Akira Hatanaka via cfe-commits
ahatanak updated this revision to Diff 39702. ahatanak added a comment. I ended up making a few more changes based on the feedback on the not_tail_called attribute: - Changed wording in docs and added a code example. - Made changes to detect both (naked,disable_tail_calls) and (disable_tail_cal

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-06 Thread Akira Hatanaka via cfe-commits
ahatanak added a comment. I intend to change the documentation, but other than that there should be no changes. I'll upload a rebased patch after I commit the other tail call patches. http://reviews.llvm.org/D12547 ___ cfe-commits mailing list cfe

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-11-06 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment. Are you still looking for review on this patch, or are you intending to make modifications based on further discussion from the other tail call attribute? http://reviews.llvm.org/D12547 ___ cfe-commits mailing list cf

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-16 Thread Akira Hatanaka via cfe-commits
ahatanak updated this revision to Diff 34919. ahatanak added a comment. Made the following changes to address review comments: - Use checkAttrMutualExclusion. - Removed spaces in the check strings in test case attr-disable-tail-calls.c. http://reviews.llvm.org/D12547 Files: include/clang/Bas

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-16 Thread Akira Hatanaka via cfe-commits
On Wed, Sep 16, 2015 at 8:05 AM, Aaron Ballman wrote: > On Wed, Sep 16, 2015 at 12:47 AM, Akira Hatanaka > wrote: > > ahatanak added inline comments. > > > > > > Comment at: include/clang/Basic/Attr.td:894 > > @@ -893,1 +893,3 @@ > > > > +def DisableTailCalls : InheritableAttr {

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-16 Thread Aaron Ballman via cfe-commits
On Wed, Sep 16, 2015 at 12:47 AM, Akira Hatanaka wrote: > ahatanak added inline comments. > > > Comment at: include/clang/Basic/Attr.td:894 > @@ -893,1 +893,3 @@ > > +def DisableTailCalls : InheritableAttr { > + let Spellings = [GNU<"disable_tail_calls">, > > aar

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-16 Thread Aaron Ballman via cfe-commits
On Wed, Sep 16, 2015 at 12:46 AM, Akira Hatanaka wrote: > ahatanak updated this revision to Diff 34872. > ahatanak added a comment. > > Sorry for the delay in my response. > > I had discussions with the users who requested this feature and it turns out > they were asking for two different kinds o

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-15 Thread Akira Hatanaka via cfe-commits
ahatanak added inline comments. Comment at: include/clang/Basic/Attr.td:894 @@ -893,1 +893,3 @@ +def DisableTailCalls : InheritableAttr { + let Spellings = [GNU<"disable_tail_calls">, aaron.ballman wrote: > Pardon me if this is obvious, but -- are there times w

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-15 Thread Akira Hatanaka via cfe-commits
ahatanak updated this revision to Diff 34872. ahatanak added a comment. Sorry for the delay in my response. I had discussions with the users who requested this feature and it turns out they were asking for two different kinds of attributes. They are both needed to disable tail call optimization

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-03 Thread Aaron Ballman via cfe-commits
Oops, apologies for making this harder than it needs to be. It seems phab didn't provide context to these comments. Check out the phab review link for more context, if you'd like it. ~Aaron On Thu, Sep 3, 2015 at 9:48 AM, Aaron Ballman wrote: > aaron.ballman added inline comments. > > ==

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-03 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:894 @@ -893,1 +893,3 @@ +def DisableTailCalls : InheritableAttr { + let Spellings = [GNU<"disable_tail_calls">, Pardon me if this is obvious, but -- are there times when you would want

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-02 Thread Akira Hatanaka via cfe-commits
ahatanak added inline comments. Comment at: include/clang/Basic/Attr.td:894 @@ -893,1 +893,3 @@ +def DisableTailCalls : InheritableAttr { + let Spellings = [GNU<"disable_tail_calls">]; aaron.ballman wrote: > We already have an attribute that is tangentially rel

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-02 Thread Akira Hatanaka via cfe-commits
ahatanak updated this revision to Diff 33882. http://reviews.llvm.org/D12547 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td lib/CodeGen/CGCall.cpp lib/Sema/SemaDeclAttr.cpp test/CodeGen/attr-disable-tail-calls.c Index: test/CodeGen/attr-disable-tail-calls.c

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-02 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:894 @@ -893,1 +893,3 @@ +def DisableTailCalls : InheritableAttr { + let Spellings = [GNU<"disable_tail_calls">]; We already have an attribute that is tangentially related: OptimizeNone

[PATCH] D12547: Add support for function attribute "disable_tail_calls"

2015-09-01 Thread Akira Hatanaka via cfe-commits
ahatanak created this revision. ahatanak added a subscriber: cfe-commits. There have been requests for a function attribute that disables tail call optimizations in the backend. This patch defines such an attribute. http://reviews.llvm.org/D12547 Files: include/clang/Basic/Attr.td lib/CodeG