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
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
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
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
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
___
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
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
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:
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
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
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
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
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
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 {
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
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
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
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
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.
>
> ==
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
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
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
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
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
24 matches
Mail list logo