ychen created this revision.
ychen added reviewers: nikic, ChuanqiXu.
Herald added subscribers: jdoerfert, hiraditya.
Herald added a project: All.
ychen requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
It is illegal to merge two `llvm.coro.save` calls unless their
`llvm.coro.suspend` users are also merged. Marks it "nomerge" for
the moment.
This reverts D129025 <https://reviews.llvm.org/D129025>.
Alternative to D129025 <https://reviews.llvm.org/D129025>, which affects other
token type users like WinEH.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D129530
Files:
clang/test/CodeGenCoroutines/coro-attributes.cpp
llvm/docs/Coroutines.rst
llvm/include/llvm/IR/Intrinsics.td
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/Transforms/SimplifyCFG/hoist-skip-token.ll
Index: llvm/test/Transforms/SimplifyCFG/hoist-skip-token.ll
===================================================================
--- llvm/test/Transforms/SimplifyCFG/hoist-skip-token.ll
+++ llvm/test/Transforms/SimplifyCFG/hoist-skip-token.ll
@@ -1,7 +1,7 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt < %s -passes='simplifycfg<hoist-common-insts>' -S | FileCheck %s
-declare token @llvm.coro.save(ptr)
+declare token @llvm.coro.save(ptr) #0
declare i8 @llvm.coro.suspend(token, i1)
define void @f(i32 %x) {
@@ -37,3 +37,5 @@
coro.ret:
ret void
}
+
+attributes #0 = { nomerge }
Index: llvm/lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1499,10 +1499,6 @@
if (I1->isTerminator())
goto HoistTerminator;
- // Hoisting token-returning instructions would obscure the origin.
- if (I1->getType()->isTokenTy())
- return Changed;
-
// If we're going to hoist a call, make sure that the two instructions
we're
// commoning/hoisting are both marked with musttail, or neither of them is
// marked as such. Otherwise, we might end up in a situation where we hoist
Index: llvm/include/llvm/IR/Intrinsics.td
===================================================================
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -1308,7 +1308,7 @@
def int_coro_size : Intrinsic<[llvm_anyint_ty], [], [IntrNoMem]>;
def int_coro_align : Intrinsic<[llvm_anyint_ty], [], [IntrNoMem]>;
-def int_coro_save : Intrinsic<[llvm_token_ty], [llvm_ptr_ty], []>;
+def int_coro_save : Intrinsic<[llvm_token_ty], [llvm_ptr_ty], [IntrNoMerge]>;
def int_coro_suspend : Intrinsic<[llvm_i8_ty], [llvm_token_ty, llvm_i1_ty],
[]>;
def int_coro_suspend_retcon : Intrinsic<[llvm_any_ty], [llvm_vararg_ty], []>;
def int_coro_prepare_retcon : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty],
Index: llvm/docs/Coroutines.rst
===================================================================
--- llvm/docs/Coroutines.rst
+++ llvm/docs/Coroutines.rst
@@ -1555,7 +1555,9 @@
The '``llvm.coro.save``' marks the point where a coroutine need to update its
state to prepare for resumption to be considered suspended (and thus eligible
-for resumption).
+for resumption). It is illegal to merge two '``llvm.coro.save``' calls unless
their
+'``llvm.coro.suspend``' users are also merged. So '``llvm.coro.save``' is
currently
+tagged with the `no_merge` function attribute.
Arguments:
""""""""""
Index: clang/test/CodeGenCoroutines/coro-attributes.cpp
===================================================================
--- clang/test/CodeGenCoroutines/coro-attributes.cpp
+++ clang/test/CodeGenCoroutines/coro-attributes.cpp
@@ -14,7 +14,9 @@
};
// CHECK: void @_Z3foov() #[[FOO_ATTR_NUM:[0-9]+]]
+// CHECK: declare token @llvm.coro.save(ptr) #[[SAVE_ATTR_NUM:[0-9]+]]
// CHECK: attributes #[[FOO_ATTR_NUM]] = { {{.*}} presplitcoroutine
+// CHECK: attributes #[[SAVE_ATTR_NUM]] = { {{.*}}nomerge
coro foo() {
co_await suspend_always{};
}
Index: llvm/test/Transforms/SimplifyCFG/hoist-skip-token.ll
===================================================================
--- llvm/test/Transforms/SimplifyCFG/hoist-skip-token.ll
+++ llvm/test/Transforms/SimplifyCFG/hoist-skip-token.ll
@@ -1,7 +1,7 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt < %s -passes='simplifycfg<hoist-common-insts>' -S | FileCheck %s
-declare token @llvm.coro.save(ptr)
+declare token @llvm.coro.save(ptr) #0
declare i8 @llvm.coro.suspend(token, i1)
define void @f(i32 %x) {
@@ -37,3 +37,5 @@
coro.ret:
ret void
}
+
+attributes #0 = { nomerge }
Index: llvm/lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1499,10 +1499,6 @@
if (I1->isTerminator())
goto HoistTerminator;
- // Hoisting token-returning instructions would obscure the origin.
- if (I1->getType()->isTokenTy())
- return Changed;
-
// If we're going to hoist a call, make sure that the two instructions we're
// commoning/hoisting are both marked with musttail, or neither of them is
// marked as such. Otherwise, we might end up in a situation where we hoist
Index: llvm/include/llvm/IR/Intrinsics.td
===================================================================
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -1308,7 +1308,7 @@
def int_coro_size : Intrinsic<[llvm_anyint_ty], [], [IntrNoMem]>;
def int_coro_align : Intrinsic<[llvm_anyint_ty], [], [IntrNoMem]>;
-def int_coro_save : Intrinsic<[llvm_token_ty], [llvm_ptr_ty], []>;
+def int_coro_save : Intrinsic<[llvm_token_ty], [llvm_ptr_ty], [IntrNoMerge]>;
def int_coro_suspend : Intrinsic<[llvm_i8_ty], [llvm_token_ty, llvm_i1_ty], []>;
def int_coro_suspend_retcon : Intrinsic<[llvm_any_ty], [llvm_vararg_ty], []>;
def int_coro_prepare_retcon : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty],
Index: llvm/docs/Coroutines.rst
===================================================================
--- llvm/docs/Coroutines.rst
+++ llvm/docs/Coroutines.rst
@@ -1555,7 +1555,9 @@
The '``llvm.coro.save``' marks the point where a coroutine need to update its
state to prepare for resumption to be considered suspended (and thus eligible
-for resumption).
+for resumption). It is illegal to merge two '``llvm.coro.save``' calls unless their
+'``llvm.coro.suspend``' users are also merged. So '``llvm.coro.save``' is currently
+tagged with the `no_merge` function attribute.
Arguments:
""""""""""
Index: clang/test/CodeGenCoroutines/coro-attributes.cpp
===================================================================
--- clang/test/CodeGenCoroutines/coro-attributes.cpp
+++ clang/test/CodeGenCoroutines/coro-attributes.cpp
@@ -14,7 +14,9 @@
};
// CHECK: void @_Z3foov() #[[FOO_ATTR_NUM:[0-9]+]]
+// CHECK: declare token @llvm.coro.save(ptr) #[[SAVE_ATTR_NUM:[0-9]+]]
// CHECK: attributes #[[FOO_ATTR_NUM]] = { {{.*}} presplitcoroutine
+// CHECK: attributes #[[SAVE_ATTR_NUM]] = { {{.*}}nomerge
coro foo() {
co_await suspend_always{};
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
