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

[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 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