[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">];
----------------
We already have an attribute that is tangentially related: OptimizeNone. Would it make sense to have that attribute take arguments that control what optimizations are enabled/disabled? How should these two attributes (and the resulting functionality) interact? For instance, if I specify optnone on a function, should that also disable tail call optimizations by creating an implicit instance of this new attribute?

================
Comment at: include/clang/Basic/Attr.td:895
@@ +894,3 @@
+def DisableTailCalls : InheritableAttr {
+  let Spellings = [GNU<"disable_tail_calls">];
+  let Subjects = SubjectList<[Function]>;
----------------
Do we also want a C++ spelling for this attribute, under the clang namespace?

================
Comment at: include/clang/Basic/Attr.td:896
@@ +895,3 @@
+  let Spellings = [GNU<"disable_tail_calls">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
----------------
Should this also apply to Objective-C methods?

================
Comment at: include/clang/Basic/Attr.td:897
@@ +896,3 @@
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}
----------------
Please document this attribute.

================
Comment at: lib/CodeGen/CGCall.cpp:1477
@@ -1476,3 +1476,3 @@
 
-    FuncAttrs.addAttribute("disable-tail-calls",
-                           llvm::toStringRef(CodeGenOpts.DisableTailCalls));
+    if ((TargetDecl && TargetDecl->hasAttr<DisableTailCallsAttr>()) ||
+        CodeGenOpts.DisableTailCalls)
----------------
Better to move this into the block that already tests TargetDecl (around line 1403 or so).

================
Comment at: lib/CodeGen/CGCall.cpp:1480
@@ +1479,3 @@
+      FuncAttrs.addAttribute("disable-tail-calls", "true");
+    else
+      FuncAttrs.addAttribute("disable-tail-calls", "false");
----------------
Can do this in a single line with ?:

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4882
@@ +4881,3 @@
+  case AttributeList::AT_DisableTailCalls:
+    handleSimpleAttribute<DisableTailCallsAttr>(S, D, Attr);
+    break;
----------------
Are there semantic checks we would like to perform on the declaration for nonsense uses of this attribute, such as functions marked [[noreturn]], etc?


http://reviews.llvm.org/D12547