[Coroutines] Part12: Handle alloca address-taken

[GorNishanov]

Summary

Move early uses of spilled variables after CoroBegin.

For example, if a parameter had address taken, we may end up with the code
like:

define @f(i32 %n) {
  %n.addr = alloca i32
  store %n, %n.addr
  ...
  call @coro.begin

This patch fixes the problem by moving uses of spilled variables after CoroBegin.

[GorNishanov]

replaced assert with report_fatal_error when we run into a case which we cannot handle

[GorNishanov]

Revised condition:

if (DT.dominates(I, CoroBegin))

to

if (!DT.dominates(CoroBegin, I))

to cover more cases where move is needed

[majnemer]

LGTM

lib/Transforms/Coroutines/CoroFrame.cpp

for (auto const &E : Spills) {
     }
 
     // Replace all uses of CurrentDef in the current instruction with the
     // CurrentMaterialization for the block.
     E.user()->replaceUsesOfWith(CurrentDef, CurrentMaterialization);
   }
 }
 
+// Move early uses of spilled variable after CoroBegin.
+// For example, if a parameter had address taken, we may end up with the code
+// like:
+//        define @f(i32 %n) {
+//          %n.addr = alloca i32
+//          store %n, %n.addr
+//          ...
+//          call @coro.begin
+//    we need to move the store after coro.begin
+static void moveSpillUsesAfterCoroBegin(Function &F, SpillInfo const &Spills,
+                                        CoroBeginInst *CoroBegin) {
+  DominatorTree DT(F);
+  SmallVector<Instruction *, 8> NeedsMoving;
+
+  Value *CurrentValue = nullptr;
+
+  for (auto const &E : Spills) {
+    if (CurrentValue == E.def())
+      continue;
+
+    CurrentValue = E.def();
+
+    for (User *U : CurrentValue->users()) {
+      Instruction *I = cast<Instruction>(U);
+      if (DT.dominates(I, CoroBegin)) {
+        // TODO: Make this more robust. Currently if we run into a situation
+        // where simple instruction move won't work we panic and
+        // report_fatal_error.
+        for (User *UI : I->users()) {
+          if (!DT.dominates(CoroBegin, cast<Instruction>(UI)))
+            report_fatal_error("cannot move instruction since its users are not"
+                               " dominated by CoroBegin");
+        }
+
+        DEBUG(dbgs() << "will move: " << *I << "\n");
+        NeedsMoving.push_back(I);
+      }
+    }
+  }
+
+  Instruction *InsertPt = CoroBegin->getNextNode();
+  for (Instruction *I : NeedsMoving)
+    I->moveBefore(InsertPt);
+}
+
 // Splits the block at a particular instruction unless it is the first
 // instruction in the block with a single predecessor.
 static BasicBlock *splitBlockIfNotFirst(Instruction *I, const Twine &Name) {
   auto *BB = I->getParent();
   if (&BB->front() == I) {
     if (BB->getSinglePredecessor()) {
       BB->setName(Name);
       return BB;
for (User *U : I.users())
               "token definition is separated from the use by a suspend point");
         assert(!materializable(I) &&
                "rewriteMaterializable did not do its job");
         Spills.emplace_back(&I, U);
       }
   }
   std::sort(Spills.begin(), Spills.end());
   DEBUG(dump("Spills", Spills));
-
+  moveSpillUsesAfterCoroBegin(F, Spills, Shape.CoroBegin);
   Shape.FrameTy = buildFrameType(F, Shape, Spills);
   Shape.FramePtr = insertSpills(Spills, Shape);
 }

test/Transforms/Coroutines/ArgAddr.ll

+; Need to move users of allocas that were moved into the coroutine frame after
+; coro.begin.
+; RUN: opt < %s -O2 -enable-coroutines -S | FileCheck %s
+
+define nonnull i8* @f(i32 %n) {
+entry:
+  %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null);
+  %n.addr = alloca i32
+  store i32 %n, i32* %n.addr ; this needs to go after coro.begin
+  %0 = tail call i32 @llvm.coro.size.i32()
+  %call = tail call i8* @malloc(i32 %0)
+  %1 = tail call noalias nonnull i8* @llvm.coro.begin(token %id, i8* %call)
+  %2 = bitcast i32* %n.addr to i8*
+  call void @ctor(i8* %2)
+  br label %for.cond
+
+for.cond:
+  %3 = load i32, i32* %n.addr
+  %dec = add nsw i32 %3, -1
+  store i32 %dec, i32* %n.addr
+  call void @print(i32 %3)
+  %4 = call i8 @llvm.coro.suspend(token none, i1 false)
+  %conv = sext i8 %4 to i32
+  switch i32 %conv, label %coro_Suspend [
+    i32 0, label %for.cond
+    i32 1, label %coro_Cleanup
+  ]
+
+coro_Cleanup:
+  %5 = call i8* @llvm.coro.free(token %id, i8* nonnull %1)
+  call void @free(i8* %5)
+  br label %coro_Suspend
+
+coro_Suspend:
+  call void @llvm.coro.end(i8* null, i1 false)
+  ret i8* %1
+}
+
+; CHECK-LABEL: @main
+define i32 @main() {
+entry:
+  %hdl = call i8* @f(i32 4)
+  call void @llvm.coro.resume(i8* %hdl)
+  call void @llvm.coro.resume(i8* %hdl)
+  call void @llvm.coro.destroy(i8* %hdl)
+  ret i32 0
+; CHECK:      call void @ctor
+; CHECK-NEXT: call void @print(i32 4)
+; CHECK-NEXT: call void @print(i32 3)
+; CHECK-NEXT: call void @print(i32 2)
+; CHECK:      ret i32 0
+}
+
+declare i8* @malloc(i32)
+declare void @free(i8*)
+declare void @print(i32)
+declare void @ctor(i8* nocapture readonly)
+
+declare token @llvm.coro.id(i32, i8*, i8*, i8*)
+declare i32 @llvm.coro.size.i32()
+declare i8* @llvm.coro.begin(token, i8*)
+declare i8 @llvm.coro.suspend(token, i1)
+declare i8* @llvm.coro.free(token, i8*)
+declare void @llvm.coro.end(i8*, i1)
+
+declare void @llvm.coro.resume(i8*)
+declare void @llvm.coro.destroy(i8*)