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 <aaron.ball...@gmail.com> wrote:
> 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 to
> disable tail calls but leave other optimizations in place? I'm trying to
> understand why these attributes are orthogonal.
>
> ================
> Comment at: include/clang/Basic/Attr.td:896
> @@ +895,3 @@
> + let Spellings = [GNU<"disable_tail_calls">,
> + CXX11<"clang", "disable_tail_calls">];
> + let Subjects = SubjectList<[Function, ObjCMethod]>;
> ----------------
> Should this attribute be plural? Are there multiple tail calls within a
> single method that can be optimized away?
>
> ================
> Comment at: include/clang/Basic/AttrDocs.td:1619
> @@ +1618,3 @@
> + let Content = [{
> +Use ``__attribute__((disable_tail_calls)))`` to instruct the backend not to
> do tail call optimization.
> + }];
> ----------------
> I would avoid using the exact syntax here because this is a GNU and C++
> attribute. Could just say:
>
> The ``disable_tail_calls`` attribute instructs the backend to not perform
> tail call optimization.
>
> ================
> Comment at: lib/Sema/SemaDeclAttr.cpp:4882
> @@ +4881,3 @@
> + case AttributeList::AT_DisableTailCalls:
> + handleSimpleAttribute<DisableTailCallsAttr>(S, D, Attr);
> + break;
> ----------------
> Okay, that makes sense. I can contrive examples of noreturn where TCO could
> happen, it just took me a while to think of them. ;-)
>
> What about other semantic checks, like warning the user about disabling TCO
> when TCO could never be enabled in the first place? Can you disable TCO for
> functions that are marked __attribute__((naked))? What about returns_twice?
>
> Unfortunately, we have a lot of attributes for which we've yet to write
> documentation, so you may need to look through Attr.td.
>
>
> http://reviews.llvm.org/D12547
>
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
