[PATCH] D43841: Add an option to disable tail-call optimization for escaping blocks

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

TCO is a pretty neglible optimization; its primary advantage is slightly better locality for stack memory.

I guess the more compelling argument is that non-escaping blocks can by definition only be executed with the original block-creating code still active, so someone debugging a crash downstack of a TCO'ed block will still have a pretty strong piece of context to start from.  An escaping block, meanwhile, could be executed by anything, so TCO'ing it might leave the stack trace with absolutely no hint about what's happened.

Alright, I can accept that.



================
Comment at: include/clang/Driver/Options.td:1419
+def fno_disable_tail_calls_escaping_blocks : Flag<["-"], "fno-disable-tail-calls-escaping-blocks">, Group<f_Group>, Flags<[CC1Option]>;
+def fdisable_tail_calls_escaping_blocks : Flag<["-"], "fdisable-tail-calls-escaping-blocks">, Group<f_Group>, Flags<[CC1Option]>;
 def force__cpusubtype__ALL : Flag<["-"], "force_cpusubtype_ALL">;
----------------
These are pretty unidiomatic option names.  I would suggest one of these:
  - [fixed]-fescaping-block-tail-calls[/fixed] (the default) and [fixed]-fno-escaping-block-tail-calls[/fixed]
  - [fixed]-enable-escaping-block-tail-calls[/fixed] (the default) and [fixed]-disable-escaping-block-tail-calls[/fixed]


================
Comment at: include/clang/Frontend/CodeGenOptions.def:66
+CODEGENOPT(DisableTailCallsEscapingBlocks, 1, 0) ///< Do not emit tail calls for
+                                                 ///< escaping blocks.
 CODEGENOPT(EmitDeclMetadata  , 1, 0) ///< Emit special metadata indicating what
----------------
"from" instead of "for" would be clearer, I think.


================
Comment at: lib/CodeGen/CodeGenModule.h:469
 
+  llvm::SmallPtrSet<const BlockDecl *, 1> NoEscapeBlocks;
+
----------------
This seems like something that Sema should store on the BlockDecl.


https://reviews.llvm.org/D43841