[PATCH] D87596: [Coroutines] Reuse storage for local variables with non-overlapping lifetimes

[Xun Li via Phabricator via llvm-commits]

lxfind added a comment.

Thank you for working on this optimization!

This type of optimizations is typically not enabled in O0, and we probably don't want to either. Should we gate it under the optimization level?



================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:365-369
+  DenseMap<AllocaInst *, int> AllocaIndex;
+  DenseMap<AllocaInst *, Spill *> SpillOfAllocas;
+  using AllocaSetType = SmallVector<AllocaInst *, 4>;
+  DenseMap<int, AllocaSetType> NonOverlapedAllocas;
+  DenseSet<AllocaInst *> AllocasWithField;
----------------
Related to the optimization level suggestion, if we are to gate this to optimization levels, we should also put all of these into a local analyzer class, and give it an interface to get the info of each alloca, such that the caller can be as independent from this optimization as possible.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:484
+  ///
+  /// Side Effects: Because We use `DenseMap` to record the allocas, the order
+  /// of allocas in the frame may be different with the order in the source
----------------
What is this referring to? Allocas in this function seems to be `SmallVector` type.


================
Comment at: llvm/lib/Transforms/Coroutines/CoroFrame.cpp:509-512
+    for (auto &I : instructions(&F)) {
+      auto *Suspend = dyn_cast<IntrinsicInst>(&I);
+      if (!Suspend || Suspend->getIntrinsicID() != Intrinsic::coro_suspend)
+        continue;
----------------
I believe you can obtain the list of suspends through `Shape.CoroSuspends`?


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

https://reviews.llvm.org/D87596