[clang] 822b92a - [Coroutines] Add the newly generated SCCs back to the CGSCC work queue after CoroSplit actually happened

2021-06-30 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2021-06-30T11:38:14-07:00
New Revision: 822b92aae439c4ba2946980c8a27bd2c8a62d90c

URL: 
https://github.com/llvm/llvm-project/commit/822b92aae439c4ba2946980c8a27bd2c8a62d90c
DIFF: 
https://github.com/llvm/llvm-project/commit/822b92aae439c4ba2946980c8a27bd2c8a62d90c.diff

LOG: [Coroutines] Add the newly generated SCCs back to the CGSCC work queue 
after CoroSplit actually happened

Relevant discussion can be found at: 
https://lists.llvm.org/pipermail/llvm-dev/2021-January/148197.html
In the existing design, An SCC that contains a coroutine will go through the 
folloing passes:
Inliner -> CoroSplitPass (fake) -> FunctionSimplificationPipeline -> Inliner -> 
CoroSplitPass (real) -> FunctionSimplificationPipeline

The first CoroSplitPass doesn't do anything other than putting the SCC back to 
the queue so that the entire pipeline can repeat.
As you can see, we run Inliner twice on the SCC consecutively without doing any 
real split, which is unnecessary and likely unintended.
What we really wanted is this:
Inliner -> FunctionSimplificationPipeline -> CoroSplitPass -> 
FunctionSimplificationPipeline
(note that we don't really need to run Inliner again on the ramp function after 
split).

Hence the way we do it here is to move CoroSplitPass to the end of the CGSCC 
pipeline, make it once for real, insert the newly generated SCCs (the clones) 
back to the pipeline so that they can be optimized, and also add a function 
simplification pipeline after CoroSplit to optimize the post-split ramp 
function.

This approach also conforms to how the new pass manager works instead of 
relying on an adhoc post split cleanup, making it ready for full switch to new 
pass manager eventually.

By looking at some of the changes to the tests, we can already observe that 
this changes allows for more optimizations applied to coroutines.

Reviewed By: aeubanks, ChuanqiXu

Differential Revision: https://reviews.llvm.org/D95807

Added: 


Modified: 
clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
llvm/lib/Passes/PassBuilder.cpp
llvm/lib/Transforms/Coroutines/CoroSplit.cpp
llvm/test/Transforms/Coroutines/ArgAddr.ll
llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
llvm/test/Transforms/Coroutines/coro-alloc-with-param-O2.ll
llvm/test/Transforms/Coroutines/coro-alloca-01.ll
llvm/test/Transforms/Coroutines/coro-alloca-02.ll
llvm/test/Transforms/Coroutines/coro-alloca-03.ll
llvm/test/Transforms/Coroutines/coro-alloca-04.ll
llvm/test/Transforms/Coroutines/coro-alloca-05.ll
llvm/test/Transforms/Coroutines/coro-alloca-06.ll
llvm/test/Transforms/Coroutines/coro-alloca-07.ll
llvm/test/Transforms/Coroutines/coro-alloca-08.ll
llvm/test/Transforms/Coroutines/coro-async.ll
llvm/test/Transforms/Coroutines/coro-byval-param.ll
llvm/test/Transforms/Coroutines/coro-catchswitch-cleanuppad.ll
llvm/test/Transforms/Coroutines/coro-catchswitch.ll
llvm/test/Transforms/Coroutines/coro-debug.ll
llvm/test/Transforms/Coroutines/coro-eh-aware-edge-split-00.ll
llvm/test/Transforms/Coroutines/coro-eh-aware-edge-split-01.ll
llvm/test/Transforms/Coroutines/coro-eh-aware-edge-split-02.ll
llvm/test/Transforms/Coroutines/coro-frame-arrayalloca.ll
llvm/test/Transforms/Coroutines/coro-frame-reuse-alloca-00.ll
llvm/test/Transforms/Coroutines/coro-frame-reuse-alloca-01.ll
llvm/test/Transforms/Coroutines/coro-frame-reuse-alloca-02.ll
llvm/test/Transforms/Coroutines/coro-frame-reuse-alloca-03.ll
llvm/test/Transforms/Coroutines/coro-frame-reuse-alloca-04.ll
llvm/test/Transforms/Coroutines/coro-frame-reuse-alloca-05.ll
llvm/test/Transforms/Coroutines/coro-frame-unreachable.ll
llvm/test/Transforms/Coroutines/coro-frame.ll
llvm/test/Transforms/Coroutines/coro-materialize.ll
llvm/test/Transforms/Coroutines/coro-padding.ll
llvm/test/Transforms/Coroutines/coro-param-copy.ll
llvm/test/Transforms/Coroutines/coro-retcon-alloca.ll
llvm/test/Transforms/Coroutines/coro-retcon-frame.ll
llvm/test/Transforms/Coroutines/coro-retcon-once-value.ll
llvm/test/Transforms/Coroutines/coro-retcon-once-value2.ll
llvm/test/Transforms/Coroutines/coro-retcon-resume-values.ll
llvm/test/Transforms/Coroutines/coro-retcon-resume-values2.ll
llvm/test/Transforms/Coroutines/coro-retcon-unreachable.ll
llvm/test/Transforms/Coroutines/coro-retcon-value.ll
llvm/test/Transforms/Coroutines/coro-retcon.ll
llvm/test/Transforms/Coroutines/coro-spill-after-phi.ll
llvm/test/Transforms/Coroutines/coro-spill-corobegin.ll
llvm/test/Transforms/Coroutines/coro-spill-defs-before-corobegin.ll
llvm/test/Transforms/Coroutines/coro-spill-promise.ll
llvm/test/Transforms/Coroutines/coro-split-00.ll
llvm/test/Transforms/Coroutines/coro-split-02.ll
llvm/test/Transforms/Coroutines/coro-split-alloc.ll
llvm/test/Transforms/Coroutines/coro-split-dbg.ll

[clang] 31eb696 - [Coroutines] Remove CoroElide from O0 pipeline

2021-06-28 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2021-06-28T19:28:27-07:00
New Revision: 31eb696fc4cd3b1ed8054d88af54f214c0f92989

URL: 
https://github.com/llvm/llvm-project/commit/31eb696fc4cd3b1ed8054d88af54f214c0f92989
DIFF: 
https://github.com/llvm/llvm-project/commit/31eb696fc4cd3b1ed8054d88af54f214c0f92989.diff

LOG: [Coroutines] Remove CoroElide from O0 pipeline

CoroElide pass works only when a post-split coroutine is inlined into another 
post-split coroutine.
In O0, there is no inlining after CoroSplit, and hence no CoroElide can happen.
It's useless to put CoroElide pass in the O0 pipeline and it will never be 
triggered (unless I miss anything).

Differential Revision: https://reviews.llvm.org/D105066

Added: 


Modified: 
clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
llvm/lib/Passes/PassBuilder.cpp
llvm/test/Transforms/Coroutines/smoketest.ll

Removed: 




diff  --git a/clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp 
b/clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
index 83f8121296690..91e0fb3042b9d 100644
--- a/clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
+++ b/clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
@@ -3,23 +3,23 @@
 
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
\
 // RUN:   -fexperimental-new-pass-manager -fdebug-pass-manager -fcoroutines-ts 
\
-// RUN:   -O0 %s 2>&1 | FileCheck %s
+// RUN:   -O0 %s 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm-bc -o /dev/null 
\
 // RUN:   -fexperimental-new-pass-manager -fdebug-pass-manager -fcoroutines-ts 
\
-// RUN:   -O1 %s 2>&1 | FileCheck %s
+// RUN:   -O1 %s 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 //
-// CHECK: Running pass:{{.*}}CoroEarlyPass
+// CHECK-ALL: Running pass:{{.*}}CoroEarlyPass
 //
 // The first coro-split pass enqueues a second run of the entire CGSCC 
pipeline.
-// CHECK: Running pass: CoroSplitPass on (_Z3foov)
-// CHECK: Running pass:{{.*}}CoroElidePass{{.*}} on {{.*}}_Z3foov{{.*}}
+// CHECK-ALL: Running pass: CoroSplitPass on (_Z3foov)
+// CHECK-OPT: Running pass:{{.*}}CoroElidePass{{.*}} on {{.*}}_Z3foov{{.*}}
 //
 // The second coro-split pass splits coroutine 'foo' into funclets
 // 'foo.resume', 'foo.destroy', and 'foo.cleanup'.
-// CHECK: Running pass: CoroSplitPass on (_Z3foov)
-// CHECK: Running pass:{{.*}}CoroElidePass{{.*}} on {{.*}}_Z3foov{{.*}}
+// CHECK-ALL: Running pass: CoroSplitPass on (_Z3foov)
+// CHECK-OPT: Running pass:{{.*}}CoroElidePass{{.*}} on {{.*}}_Z3foov{{.*}}
 //
-// CHECK: Running pass:{{.*}}CoroCleanupPass
+// CHECK-ALL: Running pass:{{.*}}CoroCleanupPass
 
 namespace std {
 namespace experimental {

diff  --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp
index 49f6c1049625f..2db8b451bf16d 100644
--- a/llvm/lib/Passes/PassBuilder.cpp
+++ b/llvm/lib/Passes/PassBuilder.cpp
@@ -1986,7 +1986,6 @@ ModulePassManager 
PassBuilder::buildO0DefaultPipeline(OptimizationLevel Level,
 
 CGSCCPassManager CGPM;
 CGPM.addPass(CoroSplitPass());
-CGPM.addPass(createCGSCCToFunctionPassAdaptor(CoroElidePass()));
 MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(std::move(CGPM)));
 
 MPM.addPass(createModuleToFunctionPassAdaptor(CoroCleanupPass()));

diff  --git a/llvm/test/Transforms/Coroutines/smoketest.ll 
b/llvm/test/Transforms/Coroutines/smoketest.ll
index bb8d26783ca9d..bd122ff00180c 100644
--- a/llvm/test/Transforms/Coroutines/smoketest.ll
+++ b/llvm/test/Transforms/Coroutines/smoketest.ll
@@ -2,21 +2,21 @@
 ; levels and -enable-coroutines adds coroutine passes to the pipeline.
 ;
 ; RUN: opt < %s -disable-output -passes='default' -enable-coroutines \
-; RUN: -debug-pass-manager 2>&1 | FileCheck %s
+; RUN: -debug-pass-manager 2>&1 | FileCheck %s --check-prefixes=CHECK-ALL
 ; RUN: opt < %s -disable-output -passes='default' -enable-coroutines \
-; RUN: -debug-pass-manager 2>&1 | FileCheck %s
+; RUN: -debug-pass-manager 2>&1 | FileCheck %s 
--check-prefixes=CHECK-ALL,CHECK-OPT
 ; RUN: opt < %s -disable-output -passes='default' -enable-coroutines \
-; RUN: -debug-pass-manager 2>&1 | FileCheck %s
+; RUN: -debug-pass-manager 2>&1 | FileCheck %s 
--check-prefixes=CHECK-ALL,CHECK-OPT
 ; RUN: opt < %s -disable-output -passes='default' -enable-coroutines \
-; RUN: -debug-pass-manager 2>&1 | FileCheck %s
+; RUN: -debug-pass-manager 2>&1 | FileCheck %s 
--check-prefixes=CHECK-ALL,CHECK-OPT
 ; RUN: opt < %s -disable-output -debug-pass-manager \
 ; RUN: 
-passes='function(coro-early),cgscc(coro-split),function(coro-elide,coro-cleanup)'
 2>&1 \
-; RUN: | FileCheck %s
+; RUN: | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-OPT
 
-; CHECK: CoroEarlyPass
-; CHECK: CoroSplitPass
-; CHECK: CoroElidePass
-; CHECK: CoroCleanupPass
+; CHECK-ALL: CoroEarlyPass
+; CHECK-ALL: CoroSplitPass
+; CHECK-OPT: CoroElidePass
+;

[clang] 5faba87 - Revert "[Coroutines] Set presplit attribute in Clang instead of CoroEarly pass"

2021-04-18 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2021-04-18T17:22:28-07:00
New Revision: 5faba87938779c595f2b4e40f933bae6571bc421

URL: 
https://github.com/llvm/llvm-project/commit/5faba87938779c595f2b4e40f933bae6571bc421
DIFF: 
https://github.com/llvm/llvm-project/commit/5faba87938779c595f2b4e40f933bae6571bc421.diff

LOG: Revert "[Coroutines] Set presplit attribute in Clang instead of CoroEarly 
pass"

This reverts commit fa6b54c44ab1d5f579304eadb7ac8bd7e72d0e77.
The commited patch broke mlir tests. It seems that mlir tests depend on 
coroutine function properties set in CoroEarly pass.

Added: 


Modified: 
clang/lib/CodeGen/CGCoroutine.cpp
clang/test/CodeGenCoroutines/coro-always-inline.cpp
llvm/lib/Transforms/Coroutines/CoroEarly.cpp
llvm/test/Transforms/Coroutines/coro-debug-O2.ll
llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
llvm/test/Transforms/Coroutines/coro-split-01.ll
llvm/test/Transforms/Coroutines/coro-split-recursive.ll
llvm/test/Transforms/Coroutines/ex0.ll
llvm/test/Transforms/Coroutines/ex1.ll
llvm/test/Transforms/Coroutines/ex2.ll
llvm/test/Transforms/Coroutines/ex3.ll
llvm/test/Transforms/Coroutines/ex4.ll
llvm/test/Transforms/Coroutines/ex5.ll
llvm/test/Transforms/Coroutines/phi-coro-end.ll
llvm/test/Transforms/Coroutines/restart-trigger.ll

Removed: 
clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp



diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index fcf8fe062367f..ca071d3d2e80f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -558,8 +558,6 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt ) {
   CurCoro.Data->SuspendBB = RetBB;
   assert(ShouldEmitLifetimeMarkers &&
  "Must emit lifetime intrinsics for coroutines");
-  // CORO_PRESPLIT_ATTR = UNPREPARED_FOR_SPLIT
-  CurFn->addFnAttr("coroutine.presplit", "0");
 
   // Backend is allowed to elide memory allocations, to help it, emit
   // auto mem = coro.alloc() ? 0 : ... allocation code ...;

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
deleted file mode 100644
index e4aa14a6ac397..0
--- a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fexperimental-new-pass-manager -O0 %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fexperimental-new-pass-manager -fno-inline -O0 %s -o - | FileCheck 
%s
-
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -O0 %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fno-inline -O0 %s -o - | FileCheck %s
-
-namespace std {
-namespace experimental {
-
-struct handle {};
-
-struct awaitable {
-  bool await_ready() noexcept { return true; }
-  // CHECK-NOT: await_suspend
-  inline void __attribute__((__always_inline__)) await_suspend(handle) 
noexcept {}
-  bool await_resume() noexcept { return true; }
-};
-
-template 
-struct coroutine_handle {
-  static handle from_address(void *address) noexcept { return {}; }
-};
-
-template 
-struct coroutine_traits {
-  struct promise_type {
-awaitable initial_suspend() { return {}; }
-awaitable final_suspend() noexcept { return {}; }
-void return_void() {}
-T get_return_object() { return T(); }
-void unhandled_exception() {}
-  };
-};
-} // namespace experimental
-} // namespace std
-
-// CHECK-LABEL: @_Z3foov
-// CHECK-LABEL: entry:
-// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
-// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
-// CHECK: [[CAST0:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST0]])
-// CHECK: [[CAST1:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST1]])
-
-// CHECK: [[CAST2:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST2]])
-// CHECK: [[CAST3:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST3]])
-void foo() { co_return; }

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline.cpp
index 6ba5a6f124169..e4aa14a6ac397 100644
---

[clang] fa6b54c - [Coroutines] Set presplit attribute in Clang instead of CoroEarly pass

2021-04-18 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2021-04-18T15:41:09-07:00
New Revision: fa6b54c44ab1d5f579304eadb7ac8bd7e72d0e77

URL: 
https://github.com/llvm/llvm-project/commit/fa6b54c44ab1d5f579304eadb7ac8bd7e72d0e77
DIFF: 
https://github.com/llvm/llvm-project/commit/fa6b54c44ab1d5f579304eadb7ac8bd7e72d0e77.diff

LOG: [Coroutines] Set presplit attribute in Clang instead of CoroEarly pass

Presplit coroutines cannot be inlined. During AlwaysInliner we check if a 
function is a presplit coroutine, if so we skip inlining.
The presplit coroutine attributes are set in CoroEarly pass.
However in O0 pipeline, AlwaysInliner runs before CoroEarly, so the attribute 
isn't set yet and will still inline the coroutine.
This causes Clang to crash: https://bugs.llvm.org/show_bug.cgi?id=49920

To fix this, we set the attributes in the Clang front-end instead of in 
CoroEarly pass.

Reviewed By: rjmccall, ChuanqiXu

Differential Revision: https://reviews.llvm.org/D100282

Added: 
clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp

Modified: 
clang/lib/CodeGen/CGCoroutine.cpp
clang/test/CodeGenCoroutines/coro-always-inline.cpp
llvm/lib/Transforms/Coroutines/CoroEarly.cpp
llvm/test/Transforms/Coroutines/coro-debug-O2.ll
llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
llvm/test/Transforms/Coroutines/coro-split-01.ll
llvm/test/Transforms/Coroutines/coro-split-recursive.ll
llvm/test/Transforms/Coroutines/ex0.ll
llvm/test/Transforms/Coroutines/ex1.ll
llvm/test/Transforms/Coroutines/ex2.ll
llvm/test/Transforms/Coroutines/ex3.ll
llvm/test/Transforms/Coroutines/ex4.ll
llvm/test/Transforms/Coroutines/ex5.ll
llvm/test/Transforms/Coroutines/phi-coro-end.ll
llvm/test/Transforms/Coroutines/restart-trigger.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index ca071d3d2e80f..fcf8fe062367f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -558,6 +558,8 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt ) {
   CurCoro.Data->SuspendBB = RetBB;
   assert(ShouldEmitLifetimeMarkers &&
  "Must emit lifetime intrinsics for coroutines");
+  // CORO_PRESPLIT_ATTR = UNPREPARED_FOR_SPLIT
+  CurFn->addFnAttr("coroutine.presplit", "0");
 
   // Backend is allowed to elide memory allocations, to help it, emit
   // auto mem = coro.alloc() ? 0 : ... allocation code ...;

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
new file mode 100644
index 0..e4aa14a6ac397
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -fno-inline -O0 %s -o - | FileCheck 
%s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fno-inline -O0 %s -o - | FileCheck %s
+
+namespace std {
+namespace experimental {
+
+struct handle {};
+
+struct awaitable {
+  bool await_ready() noexcept { return true; }
+  // CHECK-NOT: await_suspend
+  inline void __attribute__((__always_inline__)) await_suspend(handle) 
noexcept {}
+  bool await_resume() noexcept { return true; }
+};
+
+template 
+struct coroutine_handle {
+  static handle from_address(void *address) noexcept { return {}; }
+};
+
+template 
+struct coroutine_traits {
+  struct promise_type {
+awaitable initial_suspend() { return {}; }
+awaitable final_suspend() noexcept { return {}; }
+void return_void() {}
+T get_return_object() { return T(); }
+void unhandled_exception() {}
+  };
+};
+} // namespace experimental
+} // namespace std
+
+// CHECK-LABEL: @_Z3foov
+// CHECK-LABEL: entry:
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK: [[CAST0:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST0]])
+// CHECK: [[CAST1:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST1]])
+
+// CHECK: [[CAST2:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST2]])
+// CHECK: [[CAST3:%[0-9]+]] = bitcast

[clang] c0211e8 - Revert "[Coroutines] Move CoroEarly pass to before AlwaysInliner"

2021-04-18 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2021-04-18T15:38:19-07:00
New Revision: c0211e8d7d0b797fd11543c3d3f9fecf3b2069cf

URL: 
https://github.com/llvm/llvm-project/commit/c0211e8d7d0b797fd11543c3d3f9fecf3b2069cf
DIFF: 
https://github.com/llvm/llvm-project/commit/c0211e8d7d0b797fd11543c3d3f9fecf3b2069cf.diff

LOG: Revert "[Coroutines] Move CoroEarly pass to before AlwaysInliner"

This reverts commit 2b50f5a4343f8fb06acaa5c36355bcf58092c9cd.
Forgot to update the description of the commit to sync with phabricator. Going 
to redo the commit.

Added: 


Modified: 
clang/lib/CodeGen/CGCoroutine.cpp
clang/test/CodeGenCoroutines/coro-always-inline.cpp
llvm/lib/Transforms/Coroutines/CoroEarly.cpp
llvm/test/Transforms/Coroutines/coro-debug-O2.ll
llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
llvm/test/Transforms/Coroutines/coro-split-01.ll
llvm/test/Transforms/Coroutines/coro-split-recursive.ll
llvm/test/Transforms/Coroutines/ex0.ll
llvm/test/Transforms/Coroutines/ex1.ll
llvm/test/Transforms/Coroutines/ex2.ll
llvm/test/Transforms/Coroutines/ex3.ll
llvm/test/Transforms/Coroutines/ex4.ll
llvm/test/Transforms/Coroutines/ex5.ll
llvm/test/Transforms/Coroutines/phi-coro-end.ll
llvm/test/Transforms/Coroutines/restart-trigger.ll

Removed: 
clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp



diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index fcf8fe062367f..ca071d3d2e80f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -558,8 +558,6 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt ) {
   CurCoro.Data->SuspendBB = RetBB;
   assert(ShouldEmitLifetimeMarkers &&
  "Must emit lifetime intrinsics for coroutines");
-  // CORO_PRESPLIT_ATTR = UNPREPARED_FOR_SPLIT
-  CurFn->addFnAttr("coroutine.presplit", "0");
 
   // Backend is allowed to elide memory allocations, to help it, emit
   // auto mem = coro.alloc() ? 0 : ... allocation code ...;

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
deleted file mode 100644
index e4aa14a6ac397..0
--- a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fexperimental-new-pass-manager -O0 %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fexperimental-new-pass-manager -fno-inline -O0 %s -o - | FileCheck 
%s
-
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -O0 %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fno-inline -O0 %s -o - | FileCheck %s
-
-namespace std {
-namespace experimental {
-
-struct handle {};
-
-struct awaitable {
-  bool await_ready() noexcept { return true; }
-  // CHECK-NOT: await_suspend
-  inline void __attribute__((__always_inline__)) await_suspend(handle) 
noexcept {}
-  bool await_resume() noexcept { return true; }
-};
-
-template 
-struct coroutine_handle {
-  static handle from_address(void *address) noexcept { return {}; }
-};
-
-template 
-struct coroutine_traits {
-  struct promise_type {
-awaitable initial_suspend() { return {}; }
-awaitable final_suspend() noexcept { return {}; }
-void return_void() {}
-T get_return_object() { return T(); }
-void unhandled_exception() {}
-  };
-};
-} // namespace experimental
-} // namespace std
-
-// CHECK-LABEL: @_Z3foov
-// CHECK-LABEL: entry:
-// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
-// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
-// CHECK: [[CAST0:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST0]])
-// CHECK: [[CAST1:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST1]])
-
-// CHECK: [[CAST2:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST2]])
-// CHECK: [[CAST3:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST3]])
-void foo() { co_return; }

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline.cpp
index 6ba5a6f124169..e4aa14a6ac397 100644
--- a/clang/test/CodeGenCoroutines/coro-always-inline.cpp
+++

[clang] 2b50f5a - [Coroutines] Move CoroEarly pass to before AlwaysInliner

2021-04-18 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2021-04-18T14:54:04-07:00
New Revision: 2b50f5a4343f8fb06acaa5c36355bcf58092c9cd

URL: 
https://github.com/llvm/llvm-project/commit/2b50f5a4343f8fb06acaa5c36355bcf58092c9cd
DIFF: 
https://github.com/llvm/llvm-project/commit/2b50f5a4343f8fb06acaa5c36355bcf58092c9cd.diff

LOG: [Coroutines] Move CoroEarly pass to before AlwaysInliner

Presplit coroutines cannot be inlined. During AlwaysInliner we check if a 
function is a presplit coroutine, if so we skip inlining.
The presplit coroutine attributes are set in CoroEarly pass.
However in O0 pipeline, AlwaysInliner runs before CoroEarly, so the attribute 
isn't set yet and will still inline the coroutine.
This causes Clang to crash: https://bugs.llvm.org/show_bug.cgi?id=49920

Differential Revision: https://reviews.llvm.org/D100282

Added: 
clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp

Modified: 
clang/lib/CodeGen/CGCoroutine.cpp
clang/test/CodeGenCoroutines/coro-always-inline.cpp
llvm/lib/Transforms/Coroutines/CoroEarly.cpp
llvm/test/Transforms/Coroutines/coro-debug-O2.ll
llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
llvm/test/Transforms/Coroutines/coro-split-01.ll
llvm/test/Transforms/Coroutines/coro-split-recursive.ll
llvm/test/Transforms/Coroutines/ex0.ll
llvm/test/Transforms/Coroutines/ex1.ll
llvm/test/Transforms/Coroutines/ex2.ll
llvm/test/Transforms/Coroutines/ex3.ll
llvm/test/Transforms/Coroutines/ex4.ll
llvm/test/Transforms/Coroutines/ex5.ll
llvm/test/Transforms/Coroutines/phi-coro-end.ll
llvm/test/Transforms/Coroutines/restart-trigger.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index ca071d3d2e80f..fcf8fe062367f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -558,6 +558,8 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt ) {
   CurCoro.Data->SuspendBB = RetBB;
   assert(ShouldEmitLifetimeMarkers &&
  "Must emit lifetime intrinsics for coroutines");
+  // CORO_PRESPLIT_ATTR = UNPREPARED_FOR_SPLIT
+  CurFn->addFnAttr("coroutine.presplit", "0");
 
   // Backend is allowed to elide memory allocations, to help it, emit
   // auto mem = coro.alloc() ? 0 : ... allocation code ...;

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
new file mode 100644
index 0..e4aa14a6ac397
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -fno-inline -O0 %s -o - | FileCheck 
%s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fno-inline -O0 %s -o - | FileCheck %s
+
+namespace std {
+namespace experimental {
+
+struct handle {};
+
+struct awaitable {
+  bool await_ready() noexcept { return true; }
+  // CHECK-NOT: await_suspend
+  inline void __attribute__((__always_inline__)) await_suspend(handle) 
noexcept {}
+  bool await_resume() noexcept { return true; }
+};
+
+template 
+struct coroutine_handle {
+  static handle from_address(void *address) noexcept { return {}; }
+};
+
+template 
+struct coroutine_traits {
+  struct promise_type {
+awaitable initial_suspend() { return {}; }
+awaitable final_suspend() noexcept { return {}; }
+void return_void() {}
+T get_return_object() { return T(); }
+void unhandled_exception() {}
+  };
+};
+} // namespace experimental
+} // namespace std
+
+// CHECK-LABEL: @_Z3foov
+// CHECK-LABEL: entry:
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK: [[CAST0:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST0]])
+// CHECK: [[CAST1:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST1]])
+
+// CHECK: [[CAST2:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST2]])
+// CHECK: [[CAST3:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST3]])
+void

[clang] f490a59 - [OpenMP][InstrProfiling] Fix a missing instr profiling counter

2021-03-25 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2021-03-25T13:52:36-07:00
New Revision: f490a5969bd52c8a48586f134ff8f02ccbb295b3

URL: 
https://github.com/llvm/llvm-project/commit/f490a5969bd52c8a48586f134ff8f02ccbb295b3
DIFF: 
https://github.com/llvm/llvm-project/commit/f490a5969bd52c8a48586f134ff8f02ccbb295b3.diff

LOG: [OpenMP][InstrProfiling] Fix a missing instr profiling counter

When emitting a function body there needs to be a instr profiling counter 
emitted. Otherwise instr profiling won't work for this function.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D98135

Added: 
clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c

Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index a8f21548d3e0..466ff096b585 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1034,7 +1034,7 @@ LValue 
CGOpenMPRegionInfo::getThreadIDVariableLValue(CodeGenFunction ) {
   getThreadIDVariable()->getType()->castAs());
 }
 
-void CGOpenMPRegionInfo::EmitBody(CodeGenFunction , const Stmt * /*S*/) {
+void CGOpenMPRegionInfo::EmitBody(CodeGenFunction , const Stmt *S) {
   if (!CGF.HaveInsertPoint())
 return;
   // 1.2.2 OpenMP Language Terminology
@@ -1043,6 +1043,8 @@ void CGOpenMPRegionInfo::EmitBody(CodeGenFunction , 
const Stmt * /*S*/) {
   // The point of exit cannot be a branch out of the structured block.
   // longjmp() and throw() must not violate the entry/exit criteria.
   CGF.EHStack.pushTerminate();
+  if (S)
+CGF.incrementProfileCounter(S);
   CodeGen(CGF);
   CGF.EHStack.popTerminate();
 }

diff  --git a/clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c 
b/clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c
new file mode 100644
index ..9667f9cc549d
--- /dev/null
+++ b/clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c -emit-llvm %s -triple 
x86_64-unknown-linux -o - -femit-all-decls -disable-llvm-passes 
-fprofile-instrument=clang | FileCheck %s
+// expected-no-diagnostics
+
+void sub(double *restrict a, double *restrict b, int n) {
+  int i;
+
+#pragma omp parallel for
+#pragma clang loop vectorize(disable)
+  for (i = 0; i < n; i++) {
+a[i] = a[i] + b[i];
+  }
+}
+
+// CHECK-LABEL: @.omp_outlined.(
+// CHECK-NEXT:  entry:
+// CHECK: call void @llvm.instrprof.increment(
+// CHECK:   omp.precond.then:
+// CHECK-NEXT:call void @llvm.instrprof.increment(
+// CHECK:   cond.true:
+// CEHCK-NEXT:call void @llvm.instrprof.increment(
+// CHECK:   omp.inner.for.body:
+// CHECK-NEXT:call void @llvm.instrprof.increment(



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

[clang] c7a39c8 - [Coroutine][Clang] Force emit lifetime intrinsics for Coroutines

2021-03-25 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2021-03-25T13:46:20-07:00
New Revision: c7a39c833af173a7797692757b13b4b8eb801acd

URL: 
https://github.com/llvm/llvm-project/commit/c7a39c833af173a7797692757b13b4b8eb801acd
DIFF: 
https://github.com/llvm/llvm-project/commit/c7a39c833af173a7797692757b13b4b8eb801acd.diff

LOG: [Coroutine][Clang] Force emit lifetime intrinsics for Coroutines

tl;dr Correct implementation of Corouintes requires having lifetime intrinsics 
available.

Coroutine functions are functions that can be suspended and resumed latter. To 
do so, data that need to stay alive after suspension must be put on the heap 
(i.e. the coroutine frame).
The optimizer is responsible for analyzing each AllocaInst and figure out 
whether it should be put on the stack or the frame.
In most cases, for data that we are unable to accurately analyze lifetime, we 
can just conservatively put them on the heap.
Unfortunately, there exists a few cases where certain data MUST be put on the 
stack, not on the heap. Without lifetime intrinsics, we are unable to correctly 
analyze those data's lifetime.

To dig into more details, there exists cases where at certain code points, the 
current coroutine frame may have already been destroyed. Hence no frame access 
would be allowed beyond that point.
The following is a common code pattern called "Symmetric Transfer" in coroutine:
```
auto tmp = await_suspend();
__builtin_coro_resume(tmp.address());
return;
```
In the above code example, `await_suspend()` returns a new coroutine handle, 
which we will obtain the address and then resume that coroutine. This 
essentially "transfered" from the current coroutine to a different coroutine.
During the call to `await_suspend()`, the current coroutine may be destroyed, 
which should be fine because we are not accessing any data afterwards.
However when LLVM is emitting IR for the above code, it needs to emit an 
AllocaInst for `tmp`. It will then call the `address` function on tmp. 
`address` function is a member function of coroutine, and there is no way for 
the LLVM optimizer to know that it does not capture the `tmp` pointer. So when 
the optimizer looks at it, it has to conservatively assume that `tmp` may 
escape and hence put it on the heap. Furthermore, in some cases `address` call 
would be inlined, which will generate a bunch of store/load instructions that 
move the `tmp` pointer around. Those stores will also make the compiler to 
think that `tmp` might escape.
To summarize, it's really difficult for the mid-end to figure out that the 
`tmp` data is short-lived.
I made some attempt in D98638, but it appears to be way too complex and is 
basically doing the same thing as inserting lifetime intrinsics in coroutines.

Also, for reference, we already force emitting lifetime intrinsics in O0 for 
AlwaysInliner: 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Passes/PassBuilder.cpp#L1893

Differential Revision: https://reviews.llvm.org/D99227

Added: 


Modified: 
clang/lib/CodeGen/CGCoroutine.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/CodeGenCoroutines/coro-alloc.cpp
clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp
clang/test/CodeGenCoroutines/coro-await.cpp
clang/test/CodeGenCoroutines/coro-dest-slot.cpp
clang/test/CodeGenCoroutines/coro-params.cpp
clang/test/CodeGenCoroutines/coro-symmetric-transfer-01.cpp
clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 5c57ad0685d5..038238c84046 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -556,6 +556,8 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt ) {
   {Builder.getInt32(NewAlign), NullPtr, NullPtr, NullPtr});
   createCoroData(*this, CurCoro, CoroId);
   CurCoro.Data->SuspendBB = RetBB;
+  assert(ShouldEmitLifetimeMarkers &&
+ "Must emit lifetime intrinsics for coroutines");
 
   // Backend is allowed to elide memory allocations, to help it, emit
   // auto mem = coro.alloc() ? 0 : ... allocation code ...;

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index e3fdf54716ab..600312e15ef4 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1318,10 +1318,16 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, 
llvm::Function *Fn,
 
   Stmt *Body = FD->getBody();
 
-  // Initialize helper which will detect jumps which can cause invalid lifetime
-  // markers.
-  if (Body && ShouldEmitLifetimeMarkers)
-Bypasses.Init(Body);
+  if (Body) {
+// Coroutines always emit lifetime markers.
+if (isa(Body))
+  ShouldEmitLifetimeMarkers = true;
+
+// Initialize helper which will detect jumps which can cause invalid
+// lifetime

[clang] c6c8d4a - [modules] Fix crash in call to `FunctionDecl::setPure()`

2020-11-18 Thread Xun Li via cfe-commits 

Author: Andrew Gallagher
Date: 2020-11-18T11:55:29-08:00
New Revision: c6c8d4a13ebd5ce1c3c7e8632312ab8c2dc6afa0

URL: 
https://github.com/llvm/llvm-project/commit/c6c8d4a13ebd5ce1c3c7e8632312ab8c2dc6afa0
DIFF: 
https://github.com/llvm/llvm-project/commit/c6c8d4a13ebd5ce1c3c7e8632312ab8c2dc6afa0.diff

LOG: [modules] Fix crash in call to `FunctionDecl::setPure()`

In some cases, when deserializing a `CXXMethodDecl` of a 
`CXXSpecializationTemplateDecl`,
the call to `FunctionDecl::setPure()` happens before the `DefinitionData` 
member has been
populated (which appears to happen lower down in a `mergeRedeclarable` call), 
causing a
crash (https://reviews.llvm.org/P8228).

This diff fixes this by deferring the `FunctionDecl::setPure()` till after the 
`DefinitionData` has
been filled in.

Reviewed By: lxfind

Differential Revision: https://reviews.llvm.org/D86853

Added: 
clang/test/Modules/Inputs/set-pure-crash/a.h
clang/test/Modules/Inputs/set-pure-crash/b.h
clang/test/Modules/Inputs/set-pure-crash/c.h
clang/test/Modules/Inputs/set-pure-crash/module.modulemap
clang/test/Modules/set-pure-crash.cpp

Modified: 
clang/lib/Serialization/ASTReaderDecl.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 797232885687..6bfb9bd783b5 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -868,7 +868,10 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
   FD->setInlineSpecified(Record.readInt());
   FD->setImplicitlyInline(Record.readInt());
   FD->setVirtualAsWritten(Record.readInt());
-  FD->setPure(Record.readInt());
+  // We defer calling `FunctionDecl::setPure()` here as for methods of
+  // `CXXTemplateSpecializationDecl`s, we may not have connected up the
+  // definition (which is required for `setPure`).
+  const bool Pure = Record.readInt();
   FD->setHasInheritedPrototype(Record.readInt());
   FD->setHasWrittenPrototype(Record.readInt());
   FD->setDeletedAsWritten(Record.readInt());
@@ -1015,6 +1018,10 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
   }
   }
 
+  // Defer calling `setPure` until merging above has guaranteed we've set
+  // `DefinitionData` (as this will need to access it).
+  FD->setPure(Pure);
+
   // Read in the parameters.
   unsigned NumParams = Record.readInt();
   SmallVector Params;

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/a.h 
b/clang/test/Modules/Inputs/set-pure-crash/a.h
new file mode 100644
index ..f52458e0daeb
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template 
+class Base {
+public:
+  virtual void func() = 0;
+};
+
+Base bar();

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/b.h 
b/clang/test/Modules/Inputs/set-pure-crash/b.h
new file mode 100644
index ..eef7c2b9faaa
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+  void func() {
+foo([]() {});
+  }
+};

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/c.h 
b/clang/test/Modules/Inputs/set-pure-crash/c.h
new file mode 100644
index ..d5b7cd19461f
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template 
+struct simple {
+};

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/module.modulemap 
b/clang/test/Modules/Inputs/set-pure-crash/module.modulemap
new file mode 100644
index ..50bbe84e5d07
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+  header "a.h"
+}
+
+module b {
+  header "b.h"
+}
+
+module c {
+  header "c.h"
+}

diff  --git a/clang/test/Modules/set-pure-crash.cpp 
b/clang/test/Modules/set-pure-crash.cpp
new file mode 100644
index ..197e0cb00d27
--- /dev/null
+++ b/clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = simple();



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

[clang] 19f0770 - [Coroutine][Sema] Cleanup temporaries as early as possible

2020-11-10 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2020-11-10T13:27:42-08:00
New Revision: 19f07709234304b0214f5352750e85cacfda4b36

URL: 
https://github.com/llvm/llvm-project/commit/19f07709234304b0214f5352750e85cacfda4b36
DIFF: 
https://github.com/llvm/llvm-project/commit/19f07709234304b0214f5352750e85cacfda4b36.diff

LOG: [Coroutine][Sema] Cleanup temporaries as early as possible

The original bug was discovered in T75057860. Clang front-end emits an AST that 
looks like this for an co_await expression:
|- ExprWithCleanups
  |- -CoawaitExpr
|- -MaterializeTemporaryExpr ... Awaiter
  ...
|- -CXXMemberCallExpr ... .await_ready
  ...
|- -CallExpr ... __builtin_coro_resume
  ...
|- -CXXMemberCallExpr ... .await_resume
  ...

ExprWithCleanups is responsible for cleaning up (including calling dtors) for 
the temporaries generated in the wrapping expression).
In the above structure, the __builtin_coro_resume part (which corresponds to 
the code for the suspend case in the co_await with symmetric transfer), the 
pseudocode looks like this:
  __builtin_coro_resume(
   awaiter.await_suspend(
 from_address(
   __builtin_coro_frame())).address());

One of the temporaries that's generated as part of this code is the coroutine 
handle returned from awaiter.await_suspend() call. The call returns a handle  
which is a prvalue (since it's a returned value on the fly). In order to call 
the address() method on it, it needs to be converted into an xvalue. Hence a 
materialized temp is created to hold it. This temp will need to be cleaned up 
eventually. Now, since all cleanups happen at the end of the entire co_await 
expression, which is after the  suspension point, the compiler 
will think that such a temp needs to live across suspensions, and need to be 
put on the coroutine frame, even though it's only used temporarily just to call 
address() method.
Such a phenomena not only unnecessarily increases the frame size, but can lead 
to ASAN failures, if the coroutine was already destroyed as part of the 
await_suspend() call. This is because if the coroutine was already destroyed, 
the frame no longer exists, and one can not store anything into it. But if the 
temporary object is considered to need to live on the frame, it will be stored 
into the frame after await_suspend() returns.

A fix attempt was done in https://reviews.llvm.org/D87470. Unfortunately it is 
incorrect. The reason is that cleanups in Clang works more like linearly than 
nested. There is one current state indicating whether it needs cleanup, and an 
ExprWithCleanups resets that state. This means that an ExprWithCleanups must be 
capable of cleaning up all temporaries created  in the wrapping expression, 
otherwise there will be dangling temporaries cleaned up at the wrong place.
I eventually found a walk-around (https://reviews.llvm.org/D89066) that doesn't 
break any existing tests while fixing the issue. But it targets the final 
co_await only. If we ever have a co_await that's not on the final awaiter and 
the frame gets destroyed after suspend, we are in trouble. Hence we need a 
proper fix.

This patch is the proper fix. It does the folllowing things to fully resolve 
the issue:
1. The AST has to be generated in the order according to their nesting 
relationship. We should not generate AST out of order because then the code 
generator would incorrectly track the state of temporaries and when a cleanup 
is needed. So the code in buildCoawaitCalls is reorganized so that we will be 
generating the AST for each coawait member call in order along with their child 
AST.
2. await_ready() call is wrapped with an ExprWithCleanups so that temporaries 
in it gets cleaned up as early as possible to avoid living across suspension.
3. await_suspend() call is wrapped with an ExprWithCleanups if it's not a 
symmetric transfer. In the case of a symmetric transfer, in order to maintain 
the musttail call contract, the ExprWithCleanups is wraaped before the resume 
call.
4. In the end, we mark again that it needs a cleanup, so that the entire 
CoawaitExpr will be wrapped with a ExprWithCleanups which will clean up the 
Awaiter object associated with the await expression.

Differential Revision: https://reviews.llvm.org/D90990

Added: 
clang/test/AST/coroutine-locals-cleanup.cpp
clang/test/CodeGenCoroutines/coro-symmetric-transfer-01.cpp
clang/test/CodeGenCoroutines/coro-symmetric-transfer-02.cpp

Modified: 
clang/lib/Sema/SemaCoroutine.cpp
clang/test/AST/Inputs/std-coroutine.h

Removed: 
clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp



diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 5582c728aa2d..ce4e55873734 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -398,51 +398,55 @@ static Expr *maybeTailCall(Sema , QualType RetType, 
Expr *E,

[clang] d80ecdf - [Coroutine] Rename coro-semmetric-transfer.cpp and possibly fix test failure

2020-10-12 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2020-10-12T15:29:07-07:00
New Revision: d80ecdf27faf2c45a4264064ddfd5c4524dadce4

URL: 
https://github.com/llvm/llvm-project/commit/d80ecdf27faf2c45a4264064ddfd5c4524dadce4
DIFF: 
https://github.com/llvm/llvm-project/commit/d80ecdf27faf2c45a4264064ddfd5c4524dadce4.diff

LOG: [Coroutine] Rename coro-semmetric-transfer.cpp and possibly fix test 
failure

Some tests start to fail after https://reviews.llvm.org/D89066.
It's because the size of pointers are different on different targets.
Limit the target in the command so there is no confusion.
Also noticed I had typo in the test name.
Adding disable-llvm-passes option to make the test more stable as well.

Differential Revision: https://reviews.llvm.org/D89269

Added: 
clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp

Modified: 


Removed: 
clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp



diff  --git a/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp 
b/clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
similarity index 58%
rename from clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
rename to clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
index 9833f14b273d..4f841a918bcf 100644
--- a/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
+++ b/clang/test/CodeGenCoroutines/coro-symmetric-transfer.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o - | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-O1 -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
 
 #include "Inputs/coroutine.h"
 
@@ -48,6 +48,10 @@ detached_task foo() {
   co_return;
 }
 
-// check that the lifetime of the coroutine handle used to obtain the address 
ended right away.
-// CHECK:   %{{.*}} = call i8* 
@{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"*
 nonnull %{{.*}})
-// CHECK-NEXT:  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %{{.*}})
+// check that the lifetime of the coroutine handle used to obtain the address 
is contained within single basic block.
+// CHECK-LABEL: final.suspend:
+// CHECK: %[[PTR1:.+]] = bitcast 
%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* 
%[[ADDR_TMP:.+]] to i8*
+// CHECK-NEXT:call void @llvm.lifetime.start.p0i8(i64 8, i8* %[[PTR1]])
+// CHECK: call i8* 
@{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"*
 %[[ADDR_TMP]])
+// CHECK-NEXT:%[[PTR2:.+]] = bitcast 
%"struct.std::experimental::coroutines_v1::coroutine_handle.0"* %[[ADDR_TMP]] 
to i8*
+// CHECK-NEXT:call void @llvm.lifetime.end.p0i8(i64 8, i8* %[[PTR2]])



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

[clang] dce8f2b - [Coroutine][Sema] Only tighten the suspend call temp lifetime for final awaiter

2020-10-12 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2020-10-12T12:00:20-07:00
New Revision: dce8f2bb25ea1d01533d8e602f2520492fa67259

URL: 
https://github.com/llvm/llvm-project/commit/dce8f2bb25ea1d01533d8e602f2520492fa67259
DIFF: 
https://github.com/llvm/llvm-project/commit/dce8f2bb25ea1d01533d8e602f2520492fa67259.diff

LOG: [Coroutine][Sema] Only tighten the suspend call temp lifetime for final 
awaiter

In https://reviews.llvm.org/D87470 I added the change to tighten the lifetime 
of the expression awaiter.await_suspend().address.
Howver it was incorrect. ExprWithCleanups will call the dtor and end the 
lifetime for all the temps created in the current full expr.
When this is called on a normal await call, we don't want to do that.
We only want to do this for the call on the final_awaiter, to avoid writing 
into the frame after the frame is destroyed.
This change fixes it, by checking IsImplicit.

Differential Revision: https://reviews.llvm.org/D89066

Added: 


Modified: 
clang/lib/Sema/SemaCoroutine.cpp
clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 565f907e05b2..5582c728aa2d 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -375,7 +375,7 @@ static ExprResult buildMemberCall(Sema , Expr *Base, 
SourceLocation Loc,
 // returning await_suspend that results in a guaranteed tail call to the target
 // coroutine.
 static Expr *maybeTailCall(Sema , QualType RetType, Expr *E,
-   SourceLocation Loc) {
+   SourceLocation Loc, bool IsImplicit) {
   if (RetType->isReferenceType())
 return nullptr;
   Type const *T = RetType.getTypePtr();
@@ -398,10 +398,17 @@ static Expr *maybeTailCall(Sema , QualType RetType, 
Expr *E,
diag::warn_coroutine_handle_address_invalid_return_type)
 << JustAddress->getType();
 
-  // The coroutine handle used to obtain the address is no longer needed
-  // at this point, clean it up to avoid unnecessarily long lifetime which
-  // could lead to unnecessary spilling.
-  JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
+  // After the await_suspend call on the awaiter, the coroutine may have
+  // been destroyed. In that case, we can not store anything to the frame
+  // from this point on. Hence here we wrap it immediately with a cleanup. This
+  // could have applied to all await_suspend calls. However doing so causes
+  // alive objects being destructed for reasons that need further
+  // investigations. Here we walk-around it temporarily by only doing it after
+  // the suspend call on the final awaiter (indicated by IsImplicit) where it's
+  // most common to happen.
+  // TODO: Properly clean up the temps generated by await_suspend calls.
+  if (IsImplicit)
+JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
   return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
   JustAddress);
 }
@@ -409,7 +416,8 @@ static Expr *maybeTailCall(Sema , QualType RetType, Expr 
*E,
 /// Build calls to await_ready, await_suspend, and await_resume for a co_await
 /// expression.
 static ReadySuspendResumeResult buildCoawaitCalls(Sema , VarDecl 
*CoroPromise,
-  SourceLocation Loc, Expr *E) 
{
+  SourceLocation Loc, Expr *E,
+  bool IsImplicit) {
   OpaqueValueExpr *Operand = new (S.Context)
   OpaqueValueExpr(Loc, E->getType(), VK_LValue, E->getObjectKind(), E);
 
@@ -458,7 +466,8 @@ static ReadySuspendResumeResult buildCoawaitCalls(Sema , 
VarDecl *CoroPromise,
 QualType RetType = AwaitSuspend->getCallReturnType(S.Context);
 
 // Experimental support for coroutine_handle returning await_suspend.
-if (Expr *TailCallSuspend = maybeTailCall(S, RetType, AwaitSuspend, Loc))
+if (Expr *TailCallSuspend =
+maybeTailCall(S, RetType, AwaitSuspend, Loc, IsImplicit))
   Calls.Results[ACT::ACT_Suspend] = TailCallSuspend;
 else {
   // non-class prvalues always have cv-unqualified types
@@ -870,8 +879,8 @@ ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation 
Loc, Expr *E,
   SourceLocation CallLoc = E->getExprLoc();
 
   // Build the await_ready, await_suspend, await_resume calls.
-  ReadySuspendResumeResult RSS =
-  buildCoawaitCalls(*this, Coroutine->CoroutinePromise, CallLoc, E);
+  ReadySuspendResumeResult RSS = buildCoawaitCalls(
+  *this, Coroutine->CoroutinePromise, CallLoc, E, IsImplicit);
   if (RSS.IsInvalid)
 return ExprError();
 
@@ -925,8 +934,8 @@ ExprResult Sema::BuildCoyieldExpr(SourceLocation Loc, Expr 
*E) {
 E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
 
   // Build the await_ready, await_suspend, await_resume calls.
-

[clang] df477db - [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned handle

2020-09-11 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2020-09-11T13:35:37-07:00
New Revision: df477db5f9e0ea2a4890040b65002d93e33209b0

URL: 
https://github.com/llvm/llvm-project/commit/df477db5f9e0ea2a4890040b65002d93e33209b0
DIFF: 
https://github.com/llvm/llvm-project/commit/df477db5f9e0ea2a4890040b65002d93e33209b0.diff

LOG: [Coroutine][Sema] Tighten the lifetime of symmetric transfer returned 
handle

In generating the code for symmetric transfer, a temporary object is created to 
store the returned handle from await_suspend() call of the awaiter. Previously 
this temp won't be cleaned up until very later, which ends up causing this temp 
to be spilled to the heap. However, we know that this temp will no longer be 
needed after the coro_resume call. We can clean it up right after.

Differential Revision: https://reviews.llvm.org/D87470

Added: 
clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp

Modified: 
clang/lib/Sema/SemaCoroutine.cpp
clang/test/CodeGenCoroutines/Inputs/coroutine.h

Removed: 




diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 990ab2633520..565f907e05b2 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -398,6 +398,10 @@ static Expr *maybeTailCall(Sema , QualType RetType, Expr 
*E,
diag::warn_coroutine_handle_address_invalid_return_type)
 << JustAddress->getType();
 
+  // The coroutine handle used to obtain the address is no longer needed
+  // at this point, clean it up to avoid unnecessarily long lifetime which
+  // could lead to unnecessary spilling.
+  JustAddress = S.MaybeCreateExprWithCleanups(JustAddress);
   return buildBuiltinCall(S, Loc, Builtin::BI__builtin_coro_resume,
   JustAddress);
 }

diff  --git a/clang/test/CodeGenCoroutines/Inputs/coroutine.h 
b/clang/test/CodeGenCoroutines/Inputs/coroutine.h
index 5cc78a4904aa..2dd1ce7e9735 100644
--- a/clang/test/CodeGenCoroutines/Inputs/coroutine.h
+++ b/clang/test/CodeGenCoroutines/Inputs/coroutine.h
@@ -15,7 +15,7 @@ template <> struct coroutine_handle {
 return me;
   }
   void operator()() { resume(); }
-  void *address() const { return ptr; }
+  void *address() const noexcept { return ptr; }
   void resume() const { __builtin_coro_resume(ptr); }
   void destroy() const { __builtin_coro_destroy(ptr); }
   bool done() const { return __builtin_coro_done(ptr); }

diff  --git a/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp 
b/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
new file mode 100644
index ..09205799c3f7
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-semmetric-transfer.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang -std=c++14 -fcoroutines-ts -emit-llvm -S -O1 %s -o -
+
+#include "Inputs/coroutine.h"
+
+namespace coro = std::experimental::coroutines_v1;
+
+struct detached_task {
+  struct promise_type {
+detached_task get_return_object() noexcept {
+  return 
detached_task{coro::coroutine_handle::from_promise(*this)};
+}
+
+void return_void() noexcept {}
+
+struct final_awaiter {
+  bool await_ready() noexcept { return false; }
+  coro::coroutine_handle<> 
await_suspend(coro::coroutine_handle h) noexcept {
+h.destroy();
+return {};
+  }
+  void await_resume() noexcept {}
+};
+
+void unhandled_exception() noexcept {}
+
+final_awaiter final_suspend() noexcept { return {}; }
+
+coro::suspend_always initial_suspend() noexcept { return {}; }
+  };
+
+  ~detached_task() {
+if (coro_) {
+  coro_.destroy();
+  coro_ = {};
+}
+  }
+
+  void start() && {
+auto tmp = coro_;
+coro_ = {};
+tmp.resume();
+  }
+
+  coro::coroutine_handle coro_;
+};
+
+detached_task foo() {
+  co_return;
+}
+
+// check that the lifetime of the coroutine handle used to obtain the address 
ended right away.
+// CHECK:   %{{.*}} = call i8* 
@{{.*address.*}}(%"struct.std::experimental::coroutines_v1::coroutine_handle.0"*
 nonnull %{{.*}})
+// CHECK-NEXT:  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %{{.*}})



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

[clang] 9fc8772 - clang CoverageMapping tests bot cleanup

2020-07-01 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2020-07-01T16:06:56-07:00
New Revision: 9fc877213e075a76831fe71291d7c072c64c27e3

URL: 
https://github.com/llvm/llvm-project/commit/9fc877213e075a76831fe71291d7c072c64c27e3
DIFF: 
https://github.com/llvm/llvm-project/commit/9fc877213e075a76831fe71291d7c072c64c27e3.diff

LOG: clang CoverageMapping tests bot cleanup

Summary:
D82928 generated unexpected tmp files in the CoverageMapping test directory. 
This patch cleans it up and remove the file in the test bots.
It will be revered after a week.

Reviewers: thakis

Reviewed By: thakis

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82992

Added: 


Modified: 
clang/test/CoverageMapping/coroutine.cpp

Removed: 




diff  --git a/clang/test/CoverageMapping/coroutine.cpp 
b/clang/test/CoverageMapping/coroutine.cpp
index c149eefd1f01..dc9473348fc9 100644
--- a/clang/test/CoverageMapping/coroutine.cpp
+++ b/clang/test/CoverageMapping/coroutine.cpp
@@ -1,3 +1,5 @@
+// fixme: the following line is added to cleanup bots, will be removed in 
weeks.
+// RUN: rm -f %S/coroutine.ll
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
%s -o - | FileCheck %s
 
 namespace std::experimental {



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

[clang] ddcf063 - [Coroutines] Fix test breakage in D82928

2020-07-01 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2020-07-01T11:17:21-07:00
New Revision: ddcf063dd52ff1f30ac27d6f9abce6a45685a2b2

URL: 
https://github.com/llvm/llvm-project/commit/ddcf063dd52ff1f30ac27d6f9abce6a45685a2b2
DIFF: 
https://github.com/llvm/llvm-project/commit/ddcf063dd52ff1f30ac27d6f9abce6a45685a2b2.diff

LOG: [Coroutines] Fix test breakage in D82928

Summary: The test file in D82928 generated temp files within the test 
directory, causing test failures. Fix it.

Reviewers: modocache, fhahn

Reviewed By: modocache

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82986

Added: 


Modified: 
clang/test/CoverageMapping/coroutine.cpp

Removed: 




diff  --git a/clang/test/CoverageMapping/coroutine.cpp 
b/clang/test/CoverageMapping/coroutine.cpp
index ec1d64e0f970..c149eefd1f01 100644
--- a/clang/test/CoverageMapping/coroutine.cpp
+++ b/clang/test/CoverageMapping/coroutine.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
%s -o - | FileCheck %s
 
 namespace std::experimental {
 template 



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

[clang] 565e37c - [Coroutines] Fix code coverage for coroutine

2020-07-01 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2020-07-01T10:11:40-07:00
New Revision: 565e37c7702d181804c12d36b6010c513c9b3417

URL: 
https://github.com/llvm/llvm-project/commit/565e37c7702d181804c12d36b6010c513c9b3417
DIFF: 
https://github.com/llvm/llvm-project/commit/565e37c7702d181804c12d36b6010c513c9b3417.diff

LOG: [Coroutines] Fix code coverage for coroutine

Summary:
Previously, source-based coverage analysis does not work properly for coroutine.
This patch adds processing of coroutine body and co_return in the coverage 
analysis, so that we can handle them properly.
For coroutine body, we should only look at the actual function body and ignore 
the compiler-generated things; for co_return, we need to terminate the region 
similar to return statement.
Added a test, and confirms that it now works properly. (without this patch, the 
statement after the if statement will be treated wrongly)

Reviewers: lewissbaker, modocache, junparser

Reviewed By: modocache

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82928

Added: 
clang/test/CoverageMapping/coroutine.cpp

Modified: 
clang/lib/CodeGen/CoverageMappingGen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index cdbfc88e7b70..78b268f423cb 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -908,6 +908,18 @@ struct CounterCoverageMappingBuilder
 terminateRegion(S);
   }
 
+  void VisitCoroutineBodyStmt(const CoroutineBodyStmt *S) {
+extendRegion(S);
+Visit(S->getBody());
+  }
+
+  void VisitCoreturnStmt(const CoreturnStmt *S) {
+extendRegion(S);
+if (S->getOperand())
+  Visit(S->getOperand());
+terminateRegion(S);
+  }
+
   void VisitCXXThrowExpr(const CXXThrowExpr *E) {
 extendRegion(E);
 if (E->getSubExpr())

diff  --git a/clang/test/CoverageMapping/coroutine.cpp 
b/clang/test/CoverageMapping/coroutine.cpp
new file mode 100644
index ..ec1d64e0f970
--- /dev/null
+++ b/clang/test/CoverageMapping/coroutine.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fcoroutines-ts -std=c++14 
-emit-llvm -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping 
%s | FileCheck %s
+
+namespace std::experimental {
+template 
+struct coroutine_traits;
+
+template 
+struct coroutine_handle {
+  coroutine_handle() = default;
+  static coroutine_handle from_address(void *) noexcept { return {}; }
+};
+template <>
+struct coroutine_handle {
+  static coroutine_handle from_address(void *) { return {}; }
+  coroutine_handle() = default;
+  template 
+  coroutine_handle(coroutine_handle) noexcept {}
+};
+} // namespace std::experimental
+
+struct suspend_always {
+  bool await_ready() noexcept;
+  void await_suspend(std::experimental::coroutine_handle<>) noexcept;
+  void await_resume() noexcept;
+};
+
+template <>
+struct std::experimental::coroutine_traits {
+  struct promise_type {
+int get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend() noexcept;
+void return_value(int);
+  };
+};
+
+// CHECK-LABEL: _Z2f1i:
+int f1(int x) {   // CHECK-NEXT: File 0, [[@LINE]]:15 -> [[@LINE+7]]:2 = #0
+  if (x > 42) {   // CHECK-NEXT: File 0, [[@LINE]]:7 -> [[@LINE]]:13 = #0
+++x;  // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:14 -> 
[[@LINE-1]]:15 = #1
+  } else {// CHECK-NEXT: File 0, [[@LINE-2]]:15 -> [[@LINE]]:4 = #1
+co_return x + 42; // CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> 
[[@LINE-1]]:10 = (#0 - #1)
+  }   // CHECK-NEXT: File 0, [[@LINE-2]]:10 -> [[@LINE]]:4 = 
(#0 - #1)
+  co_return x;// CHECK-NEXT: Gap,File 0, [[@LINE-1]]:4 -> [[@LINE]]:3 
= #1
+} // CHECK-NEXT: File 0, [[@LINE-1]]:3 -> [[@LINE]]:2 = #1



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

[clang] c25acec - [Coroutines] Handle dependent promise types for final_suspend non-throw check

2020-06-25 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2020-06-25T11:27:27-07:00
New Revision: c25acec84594ca15748553341969f8e579290e27

URL: 
https://github.com/llvm/llvm-project/commit/c25acec84594ca15748553341969f8e579290e27
DIFF: 
https://github.com/llvm/llvm-project/commit/c25acec84594ca15748553341969f8e579290e27.diff

LOG: [Coroutines] Handle dependent promise types for final_suspend non-throw 
check

Summary:
Check that the co_await promise.final_suspend() does not potentially throw 
again after we have resolved dependent types.
This takes care of the cases where promises types are templated.
Added test cases for this scenario and confirmed that the checks happen now.
Also run libcxx tests locally to make sure all tests pass.

Reviewers: Benabik, lewissbaker, junparser, modocache

Reviewed By: modocache

Subscribers: modocache, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82332

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 19c68423e5da..f3024efa293c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9804,6 +9804,9 @@ class Sema final {
   void CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *);
   ClassTemplateDecl *lookupCoroutineTraits(SourceLocation KwLoc,
SourceLocation FuncLoc);
+  /// Check that the expression co_await promise.final_suspend() shall not be
+  /// potentially-throwing.
+  bool checkFinalSuspendNoThrow(const Stmt *FinalSuspend);
 
   
//======//
   // OpenCL extensions.

diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index 6262f4b117d3..70b8fd282056 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -642,7 +642,6 @@ static void checkNoThrow(Sema , const Stmt *E,
   } else if (SC == Expr::CallExprClass || SC == Expr::CXXMemberCallExprClass ||
  SC == Expr::CXXOperatorCallExprClass) {
 if (!cast(E)->isTypeDependent()) {
-  // FIXME: Handle dependent types.
   checkDeclNoexcept(cast(E)->getCalleeDecl());
   auto ReturnType = 
cast(E)->getCallReturnType(S.getASTContext());
   // Check the destructor of the call return type, if any.
@@ -662,22 +661,20 @@ static void checkNoThrow(Sema , const Stmt *E,
   }
 }
 
-/// Check that the expression co_await promise.final_suspend() shall not be
-/// potentially-throwing.
-static bool checkNoThrow(Sema , const Stmt *FinalSuspend) {
+bool Sema::checkFinalSuspendNoThrow(const Stmt *FinalSuspend) {
   llvm::SmallPtrSet ThrowingDecls;
   // We first collect all declarations that should not throw but not declared
   // with noexcept. We then sort them based on the location before printing.
   // This is to avoid emitting the same note multiple times on the same
   // declaration, and also provide a deterministic order for the messages.
-  checkNoThrow(S, FinalSuspend, ThrowingDecls);
+  checkNoThrow(*this, FinalSuspend, ThrowingDecls);
   auto SortedDecls = llvm::SmallVector{ThrowingDecls.begin(),
 ThrowingDecls.end()};
   sort(SortedDecls, [](const Decl *A, const Decl *B) {
 return A->getEndLoc() < B->getEndLoc();
   });
   for (const auto *D : SortedDecls) {
-S.Diag(D->getEndLoc(), diag::note_coroutine_function_declare_noexcept);
+Diag(D->getEndLoc(), diag::note_coroutine_function_declare_noexcept);
   }
   return ThrowingDecls.empty();
 }
@@ -724,7 +721,7 @@ bool Sema::ActOnCoroutineBodyStart(Scope *SC, 
SourceLocation KWLoc,
 return true;
 
   StmtResult FinalSuspend = buildSuspends("final_suspend");
-  if (FinalSuspend.isInvalid() || !checkNoThrow(*this, FinalSuspend.get()))
+  if (FinalSuspend.isInvalid() || 
!checkFinalSuspendNoThrow(FinalSuspend.get()))
 return true;
 
   ScopeInfo->setCoroutineSuspends(InitSuspend.get(), FinalSuspend.get());

diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index ff9d4d610660..ce9c30339617 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -7630,7 +7630,8 @@ 
TreeTransform::TransformCoroutineBodyStmt(CoroutineBodyStmt *S) {
 return StmtError();
   StmtResult FinalSuspend =
   getDerived().TransformStmt(S->getFinalSuspendStmt());
-  if (FinalSuspend.isInvalid())
+  if (FinalSuspend.isInvalid() ||
+  !SemaRef.checkFinalSuspendNoThrow(FinalSuspend.get()))
 return StmtError();
   ScopeInfo->setCoroutineSuspends(InitSuspend.get(), FinalSuspend.get());
   assert(isa(InitSuspend.get()) && isa(FinalSuspend.get()));

diff  --git a/clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp

[clang] 3661595 - [Coroutines] Special handle __builtin_coro_resume for final_suspend nothrow check

2020-06-25 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2020-06-25T10:49:50-07:00
New Revision: 366159566df3a980d3e34f3ec9609e77cdb4df8b

URL: 
https://github.com/llvm/llvm-project/commit/366159566df3a980d3e34f3ec9609e77cdb4df8b
DIFF: 
https://github.com/llvm/llvm-project/commit/366159566df3a980d3e34f3ec9609e77cdb4df8b.diff

LOG: [Coroutines] Special handle __builtin_coro_resume for final_suspend 
nothrow check

Summary:
In https://reviews.llvm.org/D82029 we added the conformance check that the 
expression co_await promise.final_suspend() should not potentially throw.
As part of this expression, in cases when the await_suspend() method of the 
final suspend awaiter returns a handle, __builtin_coro_resume could be called 
on the handle to immediately resume that coroutine.
__builtin_coro_resume is not declared with noexcept and it shouldn't. We need 
to special check this case here.

Reviewers: modocache, lewissbaker, junparser

Reviewed By: lewissbaker

Subscribers: modocache, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82415

Added: 


Modified: 
clang/lib/Sema/SemaCoroutine.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCoroutine.cpp 
b/clang/lib/Sema/SemaCoroutine.cpp
index c8ca247aae83..6262f4b117d3 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -614,6 +614,17 @@ static void checkNoThrow(Sema , const Stmt *E,
 // In the case of dtor, the call to dtor is implicit and hence we should
 // pass nullptr to canCalleeThrow.
 if (Sema::canCalleeThrow(S, IsDtor ? nullptr : cast(E), D)) {
+  if (const auto *FD = dyn_cast(D)) {
+// co_await promise.final_suspend() could end up calling
+// __builtin_coro_resume for symmetric transfer if await_suspend()
+// returns a handle. In that case, even __builtin_coro_resume is not
+// declared as noexcept and may throw, it does not throw _into_ the
+// coroutine that just suspended, but rather throws back out from
+// whoever called coroutine_handle::resume(), hence we claim that
+// logically it does not throw.
+if (FD->getBuiltinID() == Builtin::BI__builtin_coro_resume)
+  return;
+  }
   if (ThrowingDecls.empty()) {
 // First time seeing an error, emit the error message.
 S.Diag(cast(S.CurContext)->getLocation(),



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

[clang] 516803d - [Coroutines] Ensure co_await promise.final_suspend() does not throw

2020-06-22 Thread Xun Li via cfe-commits 

Author: Xun Li
Date: 2020-06-22T15:01:42-07:00
New Revision: 516803dc8685ebcc5bce38b05391958ffee22643

URL: 
https://github.com/llvm/llvm-project/commit/516803dc8685ebcc5bce38b05391958ffee22643
DIFF: 
https://github.com/llvm/llvm-project/commit/516803dc8685ebcc5bce38b05391958ffee22643.diff

LOG: [Coroutines] Ensure co_await promise.final_suspend() does not throw

Summary:
This patch addresses https://bugs.llvm.org/show_bug.cgi?id=46256
The spec of coroutine requires that the expression co_­await 
promise.final_­suspend() shall not be potentially-throwing.
To check this, we recursively look at every call (including Call, MemberCall, 
OperatorCall and Constructor) in all code
generated by the final suspend, and ensure that the callees are declared with 
noexcept. We also look at any returned data
type that requires explicit destruction, and check their destructors for 
noexcept.

This patch does not check declarations with dependent types yet, which will be 
done in future patches.

Updated all tests to add noexcept to the required functions, and added a 
dedicated test for this patch.

This patch might start to cause existing codebase fail to compile because most 
people may not have been strict in tagging
all the related functions noexcept.

Reviewers: lewissbaker, modocache, junparser

Reviewed By: modocache

Subscribers: arphaman, junparser, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D82029

Added: 
clang/test/SemaCXX/coroutine-final-suspend-noexcept.cpp

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaExceptionSpec.cpp
clang/test/AST/Inputs/std-coroutine.h
clang/test/AST/coroutine-source-location-crash.cpp
clang/test/Analysis/more-dtors-cfg-output.cpp
clang/test/CodeGenCXX/ubsan-coroutines.cpp
clang/test/CodeGenCoroutines/Inputs/coroutine.h
clang/test/CodeGenCoroutines/coro-alloc.cpp
clang/test/CodeGenCoroutines/coro-always-inline.cpp
clang/test/CodeGenCoroutines/coro-await-domination.cpp
clang/test/CodeGenCoroutines/coro-await-resume-eh.cpp
clang/test/CodeGenCoroutines/coro-await.cpp
clang/test/CodeGenCoroutines/coro-dest-slot.cpp
clang/test/CodeGenCoroutines/coro-gro-nrvo.cpp
clang/test/CodeGenCoroutines/coro-newpm-pipeline.cpp
clang/test/CodeGenCoroutines/coro-params.cpp
clang/test/CodeGenCoroutines/coro-promise-dtor.cpp
clang/test/CodeGenCoroutines/coro-ret-void.cpp
clang/test/CodeGenCoroutines/coro-return-voidtype-initlist.cpp
clang/test/CodeGenCoroutines/coro-return.cpp
clang/test/CodeGenCoroutines/coro-unhandled-exception.cpp
clang/test/Index/coroutines.cpp
clang/test/SemaCXX/Inputs/std-coroutine.h
clang/test/SemaCXX/co_await-range-for.cpp
clang/test/SemaCXX/coreturn-eh.cpp
clang/test/SemaCXX/coreturn.cpp
clang/test/SemaCXX/coroutine-rvo.cpp
clang/test/SemaCXX/coroutine-unhandled_exception-warning.cpp
clang/test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
clang/test/SemaCXX/coroutines.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d6d0ccaa00be..66856834a98f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10523,7 +10523,13 @@ def err_await_suspend_invalid_return_type : Error<
 def note_await_ready_no_bool_conversion : Note<
   "return type of 'await_ready' is required to be contextually convertible to 
'bool'"
 >;
-}
+def err_coroutine_promise_final_suspend_requires_nothrow : Error<
+  "the expression 'co_await __promise.final_suspend()' is required to be 
non-throwing"
+>;
+def note_coroutine_function_declare_noexcept : Note<
+  "must be declared with 'noexcept'"
+>;
+} // end of coroutines issue category
 
 let CategoryName = "Documentation Issue" in {
 def warn_not_a_doxygen_trailing_member_comment : Warning<

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 8c8e981e6065..88dd0d453883 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1697,6 +1697,10 @@ class Sema final {
   static QualType GetTypeFromParser(ParsedType Ty,
 TypeSourceInfo **TInfo = nullptr);
   CanThrowResult canThrow(const Stmt *E);
+  /// Determine whether the callee of a particular function call can throw.
+  /// E, D and Loc are all optional.
+  static CanThrowResult canCalleeThrow(Sema , const Expr *E, const Decl *D,
+   SourceLocation Loc = SourceLocation());
   const FunctionProtoType *ResolveExceptionSpec(SourceLocation Loc,
 const FunctionProtoType *FPT);
   void UpdateExceptionSpec(FunctionDecl *FD,

diff  --git