lxfind added a comment.

@bruno  Thanks for the review!



================
Comment at: clang/lib/CodeGen/CGCoroutine.cpp:221
     CGF.EmitBlock(RealSuspendBlock);
+  } else if (ForcestackStart) {
+    Builder.CreateCall(
----------------
bruno wrote:
> ChuanqiXu wrote:
> > lxfind wrote:
> > > ChuanqiXu wrote:
> > > > ChuanqiXu wrote:
> > > > > can we rewrite it into:
> > > > > ```
> > > > > else if (SuspendRet != nullptr && 
> > > > > SuspendRet->getType()->isClassType()) {
> > > > >      // generate:
> > > > >      // llvm.coro.forcestack(SuspendRet)
> > > > > }
> > > > > ```
> > > > Sorry I find we can't did it directly. As you said, we need to traverse 
> > > > down SuspendRet. And I still think we should did it only at CodeGen 
> > > > part since it looks not so hard. I guess we could make it in above 
> > > > 10~15 lines of codes.
> > > Traversing down AST isn't the hard part. The hard part is to search the 
> > > emitted IR, and look for the temporary alloca used to store the returned 
> > > handle.
> > Yes, I get your point. If we want to traverse the emitted IR, we could only 
> > search for the use-chain backward, which is also very odd. Let's see if 
> > there is other ways to modify the ASTNodes to make it more naturally.
> I'm curious whether did you consider annotating instructions with some new 
> custom metadata instead of using intrinsics? If so, what would be the 
> tradeoff? For example, if you could conditionally attach metadata some 
> "begin" metadata here:
> 
> `auto *SaveCall = Builder.CreateCall(CoroSave, {NullPtr});`
> 
> and "end" metadata here:
> 
> `auto *SuspendResult = Builder.CreateCall(CoroSuspend, {SaveCall, 
> Builder.getInt1(IsFinalSuspend)});`
The "end" part could probably be done through metadata. But I'm not sure how to 
do it for the "begin" part. The "begin" part needs to happen after the emission 
of S.getAwaitSuspendCallExpr().


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:2085
+    if (auto *II = dyn_cast<IntrinsicInst>(&I))
+      if (II->getIntrinsicID() == llvm::Intrinsic::coro_forcestack_begin) {
+        assert(II->getNumUses() == 1 &&
----------------
bruno wrote:
> Do such intrinsics never get removed? What happens when this hits a backend?
They are added to the list of DeadInstructions after collected. So they will 
all be removed at the end of the pass.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98638/new/

https://reviews.llvm.org/D98638

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to