[PATCH] D22659: [coroutines] Part 2 of N: Adding Coroutine Intrinsics

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: include/llvm/IR/Intrinsics.td:606-607
@@ +605,4 @@
+def int_coro_alloc : Intrinsic<[llvm_ptr_ty], [], [IntrNoMem]>;
+def int_coro_begin : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty, llvm_i32_ty,
+                                llvm_ptr_ty, llvm_ptr_ty], []>;
+
----------------
What happens if there are two calls to llvm.coro.begin in a function?

================
Comment at: include/llvm/IR/Intrinsics.td:609
@@ +608,3 @@
+
+def int_coro_free : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty], [IntrNoMem]>;
+def int_coro_end : Intrinsic<[], [llvm_ptr_ty, llvm_i1_ty], []>;
----------------
I don't know if `IntrNoMem` is right here but `NoCapture<0>` is probably fine.

================
Comment at: include/llvm/IR/Intrinsics.td:618-620
@@ +617,5 @@
+
+def int_coro_param : Intrinsic<[llvm_i1_ty], [llvm_ptr_ty, llvm_ptr_ty],
+                               [IntrReadMem, ReadNone<0>, NoCapture<0>,
+                                ReadNone<1>,NoCapture<1>]>;
+
----------------
How is coro.param sensitive to non-parameter stores?

================
Comment at: include/llvm/IR/Intrinsics.td:627-629
@@ +626,5 @@
+def int_coro_done : Intrinsic<[llvm_i1_ty], [llvm_ptr_ty], [IntrReadMem]>;
+def int_coro_promise : Intrinsic<[llvm_ptr_ty],
+                                 [llvm_ptr_ty, llvm_i32_ty, llvm_i1_ty],
+                                 [IntrNoMem]>;
+
----------------
NoMem seems wrong, what is your rationale?
I think you can nocapture the pointer parameter.

================
Comment at: include/llvm/IR/Intrinsics.td:633-634
@@ +632,4 @@
+
+def int_coro_subfn_addr : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty, llvm_i8_ty],
+                                    [IntrReadMem]>;
+
----------------
Is it possible for optimizations to come across this intrinsic.  If so, what semantics does it have?


Repository:
  rL LLVM

https://reviews.llvm.org/D22659