Playing with tokens OR breaking coroutines manually

[jdoerfert]

Summary

After "breaking"* the coroutine code manually:
  - 4 of 6 tests pass,
  - one performs differently than expected and therefor fails, and
  - one crashes with a weird assumptions

All in all it seems the tests/verifier could be more rigorous to
identify broken input.

• I am not a coroutines expert, I was merely playing around with tokens to see what kind of "transformation" they prevent in this setting. While I think the code is broken it might actually be well defined.

[GorNishanov]

I agree that verifier could be more rigorous with checking that coroutine intrinsics are used properly, but, I would not want to change the tests ex0 - ex5.ll as you suggested.
ex?.ll tests are examples from the coroutine walkthrough in LLVM Coroutines doc and I would like to keep them in sync with the tutorial. They represent minimally useful, but still sensible coroutines.

I would not object of adding new tests along the lines that your suggested (possibly with verifier changes to give better diagnostics) where assumptions about coroutine structure are violated.

And I would like to add that I appreciate very much your interest in LLVM coroutines and proposing this patch.

[jdoerfert]

In D30376#692486, @GorNishanov wrote:

I agree that verifier could be more rigorous with checking that coroutine intrinsics are used properly, but, I would not want to change the tests ex0 - ex5.ll as you suggested.
ex?.ll tests are examples from the coroutine walkthrough in LLVM Coroutines doc and I would like to keep them in sync with the tutorial. They represent minimally useful, but still sensible coroutines.

I would not object of adding new tests along the lines that your suggested (possibly with verifier changes to give better diagnostics) where assumptions about coroutine structure are violated.

And I would like to add that I appreciate very much your interest in LLVM coroutines and proposing this patch.

Please do not get me wrong, I do not want to change the tests (as in the patch)! I merely want to point out that the verifier and the coverage are not that good and with the changed tests I wanted to show why.

test/Transforms/Coroutines/ex0.ll

loop:
   %0 = call i8 @llvm.coro.suspend(token none, i1 false)
   switch i8 %0, label %suspend [i8 0, label %resume 
                                 i8 1, label %cleanup]
 resume:
   %inc = add i32 %n.val, 1
   br label %loop
 
 cleanup:
-  %mem = call i8* @llvm.coro.free(token %id, i8* %hdl)
-  call void @free(i8* %mem)
+  call void @free(i8* null)
   br label %suspend
 suspend:
   call void @llvm.coro.end(i8* %hdl, i1 0)  
   ret i8* %hdl
 }
 
 ; CHECK-LABEL: @main(
 define i32 @main() {

test/Transforms/Coroutines/ex1.ll

 ; First example from Doc/Coroutines.rst (one block loop)
 ; RUN: opt < %s -O2 -enable-coroutines -S | FileCheck %s
 
 define i8* @f(i32 %n) {
 entry:
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %size = call i32 @llvm.coro.size.i32()
   %alloc = call i8* @malloc(i32 %size)
-  %hdl = call noalias i8* @llvm.coro.begin(token %id, i8* %alloc)
+  %hdl = call noalias i8* @malloc(i32 %size)
   br label %loop
 loop:
   %n.val = phi i32 [ %n, %entry ], [ %inc, %loop ]
   %inc = add nsw i32 %n.val, 1
   call void @print(i32 %n.val)
   %0 = call i8 @llvm.coro.suspend(token none, i1 false)
   switch i8 %0, label %suspend [i8 0, label %loop
                                 i8 1, label %cleanup]

test/Transforms/Coroutines/ex2.ll

 ; Second example from Doc/Coroutines.rst (custom alloc and free functions)
 ; RUN: opt < %s -O2 -enable-coroutines -S | FileCheck %s
 
 define i8* @f(i32 %n) {
 entry:
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
-  %need.dyn.alloc = call i1 @llvm.coro.alloc(token %id)
+  %id2 = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
+  %need.dyn.alloc = call i1 @llvm.coro.alloc(token %id2)
   br i1 %need.dyn.alloc, label %dyn.alloc, label %coro.begin
 dyn.alloc:
   %size = call i32 @llvm.coro.size.i32()
   %alloc = call i8* @CustomAlloc(i32 %size)
   br label %coro.begin
 coro.begin:
   %phi = phi i8* [ null, %entry ], [ %alloc, %dyn.alloc ]
   %hdl = call noalias i8* @llvm.coro.begin(token %id, i8* %phi)

test/Transforms/Coroutines/ex3.ll

 loop.resume:
   %inc = add nsw i32 %n.val, 1
   %sub = xor i32 %n.val, -1
   call void @print(i32 %sub)
   %1 = call i8 @llvm.coro.suspend(token none, i1 false)
   switch i8 %1, label %suspend [i8 0, label %loop
                                 i8 1, label %cleanup]
 cleanup:
+  %cmp = icmp slt i32 %n, 12932
+  br i1 %cmp, label %free, label %suspend
+free:
   %mem = call i8* @llvm.coro.free(token %id, i8* %hdl)
   call void @free(i8* %mem)
   br label %suspend
 suspend:
   call void @llvm.coro.end(i8* %hdl, i1 false)
   ret i8* %hdl
 }
 

test/Transforms/Coroutines/ex4.ll

 ; Fourth example from Doc/Coroutines.rst (coroutine promise)
 ; RUN: opt < %s -O2 -enable-coroutines -S | FileCheck %s
 
 define i8* @f(i32 %n) {
 entry:
   %promise = alloca i32
   %pv = bitcast i32* %promise to i8*
   %id = call token @llvm.coro.id(i32 0, i8* %pv, i8* null, i8* null)
+  %cmp = icmp slt i32 %n, 12932
+  br i1 %cmp, label %allo, label %dyn.alloc
+allo:
   %need.dyn.alloc = call i1 @llvm.coro.alloc(token %id)
   br i1 %need.dyn.alloc, label %dyn.alloc, label %coro.begin
 dyn.alloc:
   %size = call i32 @llvm.coro.size.i32()
   %alloc = call i8* @malloc(i32 %size)
   br label %coro.begin
 coro.begin:
-  %phi = phi i8* [ null, %entry ], [ %alloc, %dyn.alloc ]
+  %phi = phi i8* [ null, %allo ], [ %alloc, %dyn.alloc ]
   %hdl = call noalias i8* @llvm.coro.begin(token %id, i8* %phi)
   br label %loop
 loop:
   %n.val = phi i32 [ %n, %coro.begin ], [ %inc, %loop ]
   %inc = add nsw i32 %n.val, 1
   store i32 %n.val, i32* %promise
   %0 = call i8 @llvm.coro.suspend(token none, i1 false)
   switch i8 %0, label %suspend [i8 0, label %loop

test/Transforms/Coroutines/ex5.ll

 ; Fifth example from Doc/Coroutines.rst (final suspend)
 ; RUN: opt < %s -O2 -enable-coroutines -S | FileCheck %s
 
 define i8* @f(i32 %n) {
 entry:
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
+  %id2 = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %size = call i32 @llvm.coro.size.i32()
   %alloc = call i8* @malloc(i32 %size)
   %hdl = call noalias i8* @llvm.coro.begin(token %id, i8* %alloc)
   br label %while.cond
 while.cond:
   %n.val = phi i32 [ %n, %entry ], [ %dec, %while.body ]
   %cmp = icmp sgt i32 %n.val, 0
   br i1 %cmp, label %while.body, label %while.end
 
 while.body:
   %dec = add nsw i32 %n.val, -1
   call void @print(i32 %n.val) #4
   %s = call i8 @llvm.coro.suspend(token none, i1 false)
   switch i8 %s, label %suspend [i8 0, label %while.cond
                                 i8 1, label %cleanup]
 while.end:
   %s.final = call i8 @llvm.coro.suspend(token none, i1 true)
   switch i8 %s.final, label %suspend [i8 0, label %trap
                                       i8 1, label %cleanup]
 trap: 
   call void @llvm.trap()
   unreachable
 cleanup:
-  %mem = call i8* @llvm.coro.free(token %id, i8* %hdl)
+  %mem = call i8* @llvm.coro.free(token %id2, i8* %hdl)
   call void @free(i8* %mem)
   br label %suspend
 suspend:
   call void @llvm.coro.end(i8* %hdl, i1 false)
   ret i8* %hdl
 }
 
 declare noalias i8* @malloc(i32)