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

[JunMa via Phabricator via llvm-commits]

junparser added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Coroutines/CoroSplit.h:28-32
+  CoroSplitPass(PassBuilder::OptimizationLevel Level) {
+    /// If not both Speed level and Size level are zero,
+    /// the OptLevel should not be zero.
+    OptLevel = std::max(Level.getSpeedupLevel(), Level.getSizeLevel());
+  }
----------------
ChuanqiXu wrote:
> lxfind wrote:
> > lxfind wrote:
> > > lxfind wrote:
> > > > ChuanqiXu wrote:
> > > > > lxfind wrote:
> > > > > > This isn't needed anymore
> > > > > If CoroSplitPass don't accept PassBuilder::OptimizationLevel as a construct argument, we may need to transfer PassBuilder::OptimizationLevel to unsigned in `PassBuilder.cpp`.
> > > > Yes just call `getSpeedupLevel()` on `Level` when passing it. This optimization is independent to the size optimization flags, just speed.
> > > Well, I guess it's reasonable to run it when size optimization is turned on. Feel free to just move the call to std::max to the callsite.
> > This also gets a bit fuzzy because we are mixing speed level and size level now. It goes back to my earlier suggestion, where we may just need to use a boolean to track whether coroutine needs to be optimized. I doubt we need a fine-grained control on the optimization level for coroutines.
> Yes, I think we should do simple things simple. It is fine to control the optimization behavior by a bool condition.
Would you please change WouldOptimize  to other meaningful words.


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

https://reviews.llvm.org/D87596