Fixing some brittle coroutine tests

Beginners
1

The following commit:

has introduced tests in which there is a pattern that seems to violate the intended (documented) use of coroutine intrinsics.

Primarily, I want to know if such pattern exists “in the wild”, or is it just a simple mistake with how the test was written.

The issue is with the destroy case of the first, non-final suspend (say, in @unwind_coro_end), going into a ret instruction. It should be “returning” via llvm.coro.end instead. The CoroSplit pass converts the ret into unreachable in the .destroy function, which it is allowed to do.

A change I tried implementing to SimplifyCFG “broke” the test because it has enabled this function (unwind_coro_end.destroy) to decay fully to ret void. The test looks for something (due to a completely unrelated concern) and fails when it sees an empty function.

I can change the code to adhere to “protocol” and then this no longer occurs. I am just wondering, if it makes sense to do such changes in my SImplifyCFG PR, or do I need to open another PR with just the test changes?

As an example, here’s what’s needed to fix one of the test cases there:

--- a/llvm/test/Transforms/Coroutines/coro-split-final-suspend.ll
+++ b/llvm/test/Transforms/Coroutines/coro-split-final-suspend.ll
@@ -11,7 +11,11 @@ entry:
 init:
   %initial_suspend = call i8 @llvm.coro.suspend(token none, i1 false)
   switch i8 %initial_suspend, label %init_suspend [i8 0, label %init_resume
-                                                   i8 1, label %init_suspend]
+                                                   i8 1, label %init_destroy]
+
+init_destroy:
+  call void @print(i32 10)
+  br label %unique_fallthrough_end

 init_suspend:
   ret ptr %hdl
@@ -23,11 +27,18 @@ init_resume:
 final_suspend:
   %0 = call i8 @llvm.coro.suspend(token none, i1 true)
   switch i8 %0, label %suspend [i8 0, label %resume
-                                i8 1, label %suspend]
+                                i8 1, label %destroy]
 resume:
   invoke void @print(i32 1) to label %suspend unwind label %lpad

 suspend:
+  br label %unique_fallthrough_end
+
+destroy:
+  call void @print(i32 11)
+  br label %unique_fallthrough_end
+
+unique_fallthrough_end: ; all code paths for .destroy much reach this block!
   call i1 @llvm.coro.end(ptr %hdl, i1 false, token none)
   call void @print(i32 0)
   ret ptr %hdl

edit: to clarify, the prints of 10, 11 are added to differentiate between the two branches and ensure that no simplification logic could hope to merge them and thus eliminate the use of the index field, which is what the test is interested in checking.