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

[Aaron Ballman via cfe-commits]

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_calls]] virtual int f(int a); // is this okay?
  };
  
  struct D : B {
    int f(int a) override { // is this okay?
      return g(a); // Does this get TCO?
    }
  };
  
  void f() {
    B *b = new D;
    b->f(12);
  }


================
Comment at: include/clang/Basic/AttrDocs.td:1683
@@ +1682,3 @@
+  int foo(int a) __attribute__((disable_tail_calls)) {
+    return callee(a); // This call is not tail-call optimized.
+  }
----------------
It would be useful to have a declaration for callee() to make it obvious that it *could* have been TCOed otherwise.

================
Comment at: lib/CodeGen/CGCall.cpp:1486
@@ -1484,1 +1485,3 @@
+        (TargetDecl && TargetDecl->hasAttr<DisableTailCallsAttr>()) ||
+        CodeGenOpts.DisableTailCalls;
     FuncAttrs.addAttribute("disable-tail-calls",
----------------
I would swap the order of the checks. It's less expensive when CodeGenOpts.DisableTailCalls is true than it is to call hasAttr().


http://reviews.llvm.org/D12547