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

[Akira Hatanaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326530: Add an option to disable tail-call optimization for 
escaping blocks. (authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43841?vs=136584=136658#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43841

Files:
  cfe/trunk/include/clang/AST/Decl.h
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/CodeGenOptions.def
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaExprObjC.cpp
  cfe/trunk/lib/Sema/SemaPseudoObject.cpp
  cfe/trunk/test/CodeGenObjC/disable-tail-call-escaping-block.m
  cfe/trunk/test/Driver/fno-escaping-block-tail-calls.c

Index: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def
@@ -62,6 +62,8 @@
///< pass manager.
 CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
 CODEGENOPT(DisableTailCalls  , 1, 0) ///< Do not emit tail calls.
+CODEGENOPT(NoEscapingBlockTailCalls, 1, 0) ///< Do not emit tail calls from
+   ///< escaping blocks.
 CODEGENOPT(EmitDeclMetadata  , 1, 0) ///< Emit special metadata indicating what
  ///< Decl* various IR entities came from.
  ///< Only useful when running CodeGen as a
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1430,6 +1430,8 @@
 def fno_openmp_cuda_mode : Flag<["-"], "fno-openmp-cuda-mode">, Group, Flags<[CC1Option, NoArgumentUnused]>;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, Group;
 def foptimize_sibling_calls : Flag<["-"], "foptimize-sibling-calls">, Group;
+def fno_escaping_block_tail_calls : Flag<["-"], "fno-escaping-block-tail-calls">, Group, Flags<[CC1Option]>;
+def fescaping_block_tail_calls : Flag<["-"], "fescaping-block-tail-calls">, Group;
 def force__cpusubtype__ALL : Flag<["-"], "force_cpusubtype_ALL">;
 def force__flat__namespace : Flag<["-"], "force_flat_namespace">;
 def force__load : Separate<["-"], "force_load">;
Index: cfe/trunk/include/clang/AST/Decl.h
===
--- cfe/trunk/include/clang/AST/Decl.h
+++ cfe/trunk/include/clang/AST/Decl.h
@@ -3802,6 +3802,10 @@
   bool BlockMissingReturnType : 1;
   bool IsConversionFromLambda : 1;
 
+  /// A bit that indicates this block is passed directly to a function as a
+  /// non-escaping parameter.
+  bool DoesNotEscape : 1;
+
   /// A new[]'d array of pointers to ParmVarDecls for the formal
   /// parameters of this function.  This is null if a prototype or if there are
   /// no formals.
@@ -3821,7 +3825,7 @@
   BlockDecl(DeclContext *DC, SourceLocation CaretLoc)
   : Decl(Block, DC, CaretLoc), DeclContext(Block), IsVariadic(false),
 CapturesCXXThis(false), BlockMissingReturnType(true),
-IsConversionFromLambda(false) {}
+IsConversionFromLambda(false), DoesNotEscape(false) {}
 
 public:
   static BlockDecl *Create(ASTContext , DeclContext *DC, SourceLocation L); 
@@ -3893,6 +3897,9 @@
   bool isConversionFromLambda() const { return IsConversionFromLambda; }
   void setIsConversionFromLambda(bool val) { IsConversionFromLambda = val; }
 
+  bool doesNotEscape() const { return DoesNotEscape; }
+  void setDoesNotEscape() { DoesNotEscape = true; }
+
   bool capturesVariable(const VarDecl *var) const;
 
   void setCaptures(ASTContext , ArrayRef Captures,
Index: cfe/trunk/test/CodeGenObjC/disable-tail-call-escaping-block.m
===
--- cfe/trunk/test/CodeGenObjC/disable-tail-call-escaping-block.m
+++ cfe/trunk/test/CodeGenObjC/disable-tail-call-escaping-block.m
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fblocks -fno-escaping-block-tail-calls -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @test(
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE0:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE1:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE2:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE3:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE4:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE5:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE6:.*]] to i8*)
+
+// 

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

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Alright, this looks good to me.




Comment at: lib/Sema/SemaExpr.cpp:4846
+if (auto *BE = dyn_cast(Arg->IgnoreParenNoopCasts(Context)))
+  BE->getBlockDecl()->setDoesNotEscape();
+

ahatanak wrote:
> rjmccall wrote:
> > Can this be based on the noescape parameter bit on the function type?
> Oh that's right. It should be able to set the DoesNotEscape bit of a block 
> when it's passed to an indirect function call. I added an IRGen test case 
> that checks tail-call is enabled in such cases.
> 
Thanks!


https://reviews.llvm.org/D43841



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 136584.
ahatanak marked an inline comment as done.
ahatanak added a comment.

Check the function prototype's noescape bit.


https://reviews.llvm.org/D43841

Files:
  include/clang/AST/Decl.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/disable-tail-call-escaping-block.m
  test/Driver/fno-escaping-block-tail-calls.c

Index: test/Driver/fno-escaping-block-tail-calls.c
===
--- /dev/null
+++ test/Driver/fno-escaping-block-tail-calls.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### %s -fescaping-block-tail-calls -fno-escaping-block-tail-calls 2> %t
+// RUN: FileCheck --check-prefix=CHECK-DISABLE < %t %s
+// CHECK-DISABLE: "-fno-escaping-block-tail-calls"
+
+// RUN: %clang -### %s -fno-escaping-block-tail-calls -fescaping-block-tail-calls 2> %t
+// RUN: FileCheck --check-prefix=CHECK-NO-DISABLE < %t %s
+// CHECK-NO-DISABLE-NOT: "-fno-escaping-block-tail-calls"
Index: test/CodeGenObjC/disable-tail-call-escaping-block.m
===
--- /dev/null
+++ test/CodeGenObjC/disable-tail-call-escaping-block.m
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fblocks -fno-escaping-block-tail-calls -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @test(
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE0:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE1:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE2:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE3:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE4:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE5:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE6:.*]] to i8*)
+
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE0]]({{.*}}) #[[DISABLEATTR:.*]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE1]]({{.*}}) #[[ENABLEATTR:.*]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE2]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE3]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE4]]({{.*}}) #[[ENABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE5]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE6]]({{.*}}) #[[ENABLEATTR]] {
+
+// CHECK: attributes #[[ENABLEATTR]] = {{{.*}}"disable-tail-calls"="false"{{.*}}}
+// CHECK: attributes #[[DISABLEATTR]] = {{{.*}}"disable-tail-calls"="true"{{.*}}}
+
+typedef void (^BlockTy)(void);
+typedef void (*NoEscapeFnTy)(__attribute__((noescape)) BlockTy);
+
+void callee0(__attribute__((noescape)) BlockTy);
+void callee1(BlockTy);
+
+__attribute__((objc_root_class))
+@interface C0
+-(void)m0:(__attribute__((noescape)) BlockTy)p;
+-(void)m1:(BlockTy)p;
+@end
+
+@implementation C0
+-(void)m0:(__attribute__((noescape)) BlockTy)p {}
+-(void)m1:(BlockTy)p {}
+@end
+
+NoEscapeFnTy noescapefunc;
+
+void test(id a, C0 *c0) {
+  BlockTy b0 = ^{ (void)a; }; // disable tail-call optimization.
+  callee0(b0);
+  callee0(^{ (void)a; }); // enable tail-call optimization.
+  callee1(^{ (void)a; }); // disable tail-call optimization.
+
+  BlockTy b1 = ^{ (void)a; }; // disable tail-call optimization.
+  [c0 m0:b1];
+  [c0 m0:^{ (void)a; }]; // enable tail-call optimization.
+  [c0 m1:^{ (void)a; }]; // disable tail-call optimization.
+
+  noescapefunc(^{ (void)a; }); // enable tail-call optimization.
+}
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1613,6 +1613,11 @@
 ParmVarDecl *param = Method->parameters()[i];
 assert(argExpr && "CheckMessageArgumentTypes(): missing expression");
 
+if (param->hasAttr())
+  if (auto *BE = dyn_cast(
+  argExpr->IgnoreParenNoopCasts(Context)))
+BE->getBlockDecl()->setDoesNotEscape();
+
 // Strip the unbridged-cast placeholder expression off unless it's
 // a consumed argument.
 if (argExpr->hasPlaceholderType(BuiltinType::ARCUnbridgedCast) &&
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -4841,6 +4841,10 @@
(!Param || !Param->hasAttr()))
 CFAudited = true;
 
+  if (Proto->getExtParameterInfo(i).isNoEscape())
+if (auto *BE = dyn_cast(Arg->IgnoreParenNoopCasts(Context)))
+  BE->getBlockDecl()->setDoesNotEscape();
+
   InitializedEntity Entity =
   Param ? InitializedEntity::InitializeParameter(Context, Param,
  

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak marked an inline comment as done.
ahatanak added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:4846
+if (auto *BE = dyn_cast(Arg->IgnoreParenNoopCasts(Context)))
+  BE->getBlockDecl()->setDoesNotEscape();
+

rjmccall wrote:
> Can this be based on the noescape parameter bit on the function type?
Oh that's right. It should be able to set the DoesNotEscape bit of a block when 
it's passed to an indirect function call. I added an IRGen test case that 
checks tail-call is enabled in such cases.



https://reviews.llvm.org/D43841



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:4846
+if (auto *BE = dyn_cast(Arg->IgnoreParenNoopCasts(Context)))
+  BE->getBlockDecl()->setDoesNotEscape();
+

Can this be based on the noescape parameter bit on the function type?


https://reviews.llvm.org/D43841



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 136580.
ahatanak marked 3 inline comments as done.
ahatanak added a comment.

Address review comments.


https://reviews.llvm.org/D43841

Files:
  include/clang/AST/Decl.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/disable-tail-call-escaping-block.m
  test/Driver/fno-escaping-block-tail-calls.c

Index: test/Driver/fno-escaping-block-tail-calls.c
===
--- /dev/null
+++ test/Driver/fno-escaping-block-tail-calls.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### %s -fescaping-block-tail-calls -fno-escaping-block-tail-calls 2> %t
+// RUN: FileCheck --check-prefix=CHECK-DISABLE < %t %s
+// CHECK-DISABLE: "-fno-escaping-block-tail-calls"
+
+// RUN: %clang -### %s -fno-escaping-block-tail-calls -fescaping-block-tail-calls 2> %t
+// RUN: FileCheck --check-prefix=CHECK-NO-DISABLE < %t %s
+// CHECK-NO-DISABLE-NOT: "-fno-escaping-block-tail-calls"
Index: test/CodeGenObjC/disable-tail-call-escaping-block.m
===
--- /dev/null
+++ test/CodeGenObjC/disable-tail-call-escaping-block.m
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fblocks -fno-escaping-block-tail-calls -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @test(
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE0:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE1:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE2:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE3:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE4:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE5:.*]] to i8*)
+
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE0]]({{.*}}) #[[DISABLEATTR:.*]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE1]]({{.*}}) #[[ENABLEATTR:.*]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE2]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE3]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE4]]({{.*}}) #[[ENABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE5]]({{.*}}) #[[DISABLEATTR]] {
+
+// CHECK: attributes #[[ENABLEATTR]] = {{{.*}}"disable-tail-calls"="false"{{.*}}}
+// CHECK: attributes #[[DISABLEATTR]] = {{{.*}}"disable-tail-calls"="true"{{.*}}}
+
+typedef void (^BlockTy)(void);
+
+void callee0(__attribute__((noescape)) BlockTy);
+void callee1(BlockTy);
+
+__attribute__((objc_root_class))
+@interface C0
+-(void)m0:(__attribute__((noescape)) BlockTy)p;
+-(void)m1:(BlockTy)p;
+@end
+
+@implementation C0
+-(void)m0:(__attribute__((noescape)) BlockTy)p {}
+-(void)m1:(BlockTy)p {}
+@end
+
+void test(id a, C0 *c0) {
+  BlockTy b0 = ^{ (void)a; }; // disable tail-call optimization.
+  callee0(b0);
+  callee0(^{ (void)a; }); // enable tail-call optimization.
+  callee1(^{ (void)a; }); // disable tail-call optimization.
+
+  BlockTy b1 = ^{ (void)a; }; // disable tail-call optimization.
+  [c0 m0:b1];
+  [c0 m0:^{ (void)a; }]; // enable tail-call optimization.
+  [c0 m1:^{ (void)a; }]; // disable tail-call optimization.
+}
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1613,6 +1613,11 @@
 ParmVarDecl *param = Method->parameters()[i];
 assert(argExpr && "CheckMessageArgumentTypes(): missing expression");
 
+if (param->hasAttr())
+  if (auto *BE = dyn_cast(
+  argExpr->IgnoreParenNoopCasts(Context)))
+BE->getBlockDecl()->setDoesNotEscape();
+
 // Strip the unbridged-cast placeholder expression off unless it's
 // a consumed argument.
 if (argExpr->hasPlaceholderType(BuiltinType::ARCUnbridgedCast) &&
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -4841,6 +4841,10 @@
(!Param || !Param->hasAttr()))
 CFAudited = true;
 
+  if (Param && Param->hasAttr())
+if (auto *BE = dyn_cast(Arg->IgnoreParenNoopCasts(Context)))
+  BE->getBlockDecl()->setDoesNotEscape();
+
   InitializedEntity Entity =
   Param ? InitializedEntity::InitializeParameter(Context, Param,
  ProtoArgType)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -640,6 +640,8 @@
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
   Opts.DiscardValueNames = 

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: include/clang/Driver/Options.td:1419
+def fno_disable_tail_calls_escaping_blocks : Flag<["-"], 
"fno-disable-tail-calls-escaping-blocks">, Group, Flags<[CC1Option]>;
+def fdisable_tail_calls_escaping_blocks : Flag<["-"], 
"fdisable-tail-calls-escaping-blocks">, Group, Flags<[CC1Option]>;
 def force__cpusubtype__ALL : Flag<["-"], "force_cpusubtype_ALL">;

rjmccall wrote:
> rjmccall wrote:
> > 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]
> Wow, this is not even close to Phabricator markup, I don't know what I was 
> thinking.
I chose the first option 
"-fno-escaping-block-tail-calls/-fescaping-block-tail-calls".


https://reviews.llvm.org/D43841



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Driver/Options.td:1419
+def fno_disable_tail_calls_escaping_blocks : Flag<["-"], 
"fno-disable-tail-calls-escaping-blocks">, Group, Flags<[CC1Option]>;
+def fdisable_tail_calls_escaping_blocks : Flag<["-"], 
"fdisable-tail-calls-escaping-blocks">, Group, Flags<[CC1Option]>;
 def force__cpusubtype__ALL : Flag<["-"], "force_cpusubtype_ALL">;

rjmccall wrote:
> 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]
Wow, this is not even close to Phabricator markup, I don't know what I was 
thinking.


https://reviews.llvm.org/D43841



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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, Flags<[CC1Option]>;
+def fdisable_tail_calls_escaping_blocks : Flag<["-"], 
"fdisable-tail-calls-escaping-blocks">, 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 NoEscapeBlocks;
+

This seems like something that Sema should store on the BlockDecl.


https://reviews.llvm.org/D43841



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

This is limited to escaping blocks because disabling tail-call optimizations 
for all blocks might impact performance. The user is claiming that non-escaping 
blocks are often used in areas that are performance-sensitive (for example, 
dispatch_sync() and -[NSArray enumerateObjectsUsingBlock:] in a tight loop), so 
disabling tail-call optimization indiscriminately can cause performance 
degradation (and clients might decide not to use the command line option 
because of that).


https://reviews.llvm.org/D43841



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Can you explain the rationale for limiting this to escaping blocks in more 
depth?  That seems like an extremely orthogonal limitation; the user might be 
thinking about a very specific block and not really considering this in general.


https://reviews.llvm.org/D43841



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added a reviewer: rjmccall.

This patch adds a command line option (-fdisable-tail-calls-esca
ping-blocks) that annotates escaping block invoke functions with attribute 
"disable-tail-calls". This is an option that helps users in debugging their 
code who spend a lot of time trying to figure out where a block came from.

The user who is asking for this command line option does not want to disable 
tail-call optimization for non-escaping blocks. For example, in the following 
code, we should not disable tail-call optimization for the block that is 
directly passed to function "noescapefunc":

  void foo3(void);
  
  void foo1() {
noescapefunc(^{ foo3(); }); // do not disable tail-call.
BlockTy b = ^{ foo3(); }; // disable tail-call.
noescapefunc(b);
  }

Ideally, I think we want to avoid disabling tail-call optimization for block 
"b" too, as it doesn't escape. However, this patch doesn't do anything to avoid 
disabling tail-call optimization for the block, since that would require a more 
complex analysis.

rdar://problem/35758207


https://reviews.llvm.org/D43841

Files:
  include/clang/AST/DeclObjC.h
  include/clang/AST/Type.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/AST/DeclObjC.cpp
  lib/AST/Type.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGenObjC/disable-tail-call-escaping-block.m
  test/Driver/fdisable-tail-calls-escaping-blocks.c

Index: test/Driver/fdisable-tail-calls-escaping-blocks.c
===
--- /dev/null
+++ test/Driver/fdisable-tail-calls-escaping-blocks.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### %s -fno-disable-tail-calls-escaping-blocks -fdisable-tail-calls-escaping-blocks 2> %t
+// RUN: FileCheck --check-prefix=CHECK-DISABLE < %t %s
+// CHECK-DISABLE: "-fdisable-tail-calls-escaping-blocks"
+
+// RUN: %clang -### %s -fdisable-tail-calls-escaping-blocks -fno-disable-tail-calls-escaping-blocks 2> %t
+// RUN: FileCheck --check-prefix=CHECK-NO-DISABLE < %t %s
+// CHECK-NO-DISABLE-NOT: "-fdisable-tail-calls-escaping-blocks"
Index: test/CodeGenObjC/disable-tail-call-escaping-block.m
===
--- /dev/null
+++ test/CodeGenObjC/disable-tail-call-escaping-block.m
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fblocks -fdisable-tail-calls-escaping-blocks -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @test(
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE0:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE1:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE2:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE3:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE4:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE5:.*]] to i8*)
+
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE0]]({{.*}}) #[[DISABLEATTR:.*]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE1]]({{.*}}) #[[ENABLEATTR:.*]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE2]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE3]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE4]]({{.*}}) #[[ENABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE5]]({{.*}}) #[[DISABLEATTR]] {
+
+// CHECK: attributes #[[ENABLEATTR]] = {{{.*}}"disable-tail-calls"="false"{{.*}}}
+// CHECK: attributes #[[DISABLEATTR]] = {{{.*}}"disable-tail-calls"="true"{{.*}}}
+
+typedef void (^BlockTy)(void);
+
+void callee0(__attribute__((noescape)) BlockTy);
+void callee1(BlockTy);
+
+__attribute__((objc_root_class))
+@interface C0
+-(void)m0:(__attribute__((noescape)) BlockTy)p;
+-(void)m1:(BlockTy)p;
+@end
+
+@implementation C0
+-(void)m0:(__attribute__((noescape)) BlockTy)p {}
+-(void)m1:(BlockTy)p {}
+@end
+
+void test(id a, C0 *c0) {
+  BlockTy b0 = ^{ (void)a; }; // disable tail-call optimization.
+  callee0(b0);
+  callee0(^{ (void)a; }); // enable tail-call optimization.
+  callee1(^{ (void)a; }); // disable tail-call optimization.
+
+  BlockTy b1 = ^{ (void)a; }; // disable tail-call optimization.
+  [c0 m0:b1];
+  [c0 m0:^{ (void)a; }]; // enable tail-call optimization.
+  [c0 m1:^{ (void)a; }]; // disable tail-call optimization.
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -639,6 +639,8 @@
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
   Opts.DiscardValueNames = Args.hasArg(OPT_discard_value_names);
   Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls);
+  Opts.DisableTailCallsEscapingBlocks =
+