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

[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 when you would want to disable tail calls but leave other optimizations in place? I'm trying to understand why these attributes are orthogonal.
Yes, that's correct. The new function attribute we are adding shouldn't disable other optimizations that are normally run at -O2 or -O3.

================
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]>;
----------------
aaron.ballman wrote:
> Should this attribute be plural? Are there multiple tail calls within a single method that can be optimized away?
I named it after the existing IR function attribute and I'm guessing the plural name was chosen because functions can have multiple tail call sites. I think the singular name is better if we want this attribute to mean "disable tail call optimization".

================
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.
+  }];
----------------
aaron.ballman wrote:
> 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.
OK, done.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4882
@@ +4881,3 @@
+  case AttributeList::AT_DisableTailCalls:
+    handleSimpleAttribute<DisableTailCallsAttr>(S, D, Attr);
+    break;
----------------
aaron.ballman wrote:
> 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.
Since "naked" allows only inline assembly statements, it should be an error to have disable-tail-calls and naked on the same function. I made changes in Sema to detect that case.

My understanding is that you can't do tail call optimization on call sites of a function that calls a "return_twice" function. However, attaching "return_twice" on the calling function doesn't block tail call optimizations on the call sites inside the function.

I didn't find any other attributes that seemed incompatible with disable-tail-calls.


http://reviews.llvm.org/D12547