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

Xun Li via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 22 08:54:40 PDT 2020


lxfind accepted this revision.
lxfind added a comment.
This revision is now accepted and ready to land.

Overall LGTM. Thank you for addressing the feedback!
One last thing, the optimization level code can be further simplified. Since you are obtaining the `OptLevel` from the `PassManagerBuilder`, which is an integer. I feel it would be easier if you just stick with an integer optlevel instead of converting it to `OptimizationLevel`. The reason `OptimizationLevel` exists is to be able to model both OptLevel and SizeLevel at the same time, but here you don't really care about the SizeLevel. So I would suggest just use an int for it and save all the trouble converting.



================
Comment at: llvm/lib/Passes/PassBuilder.cpp:1915
 }
-
 } // namespace
----------------
Nit: Unnecessary change


================
Comment at: llvm/lib/Transforms/Coroutines/Coroutines.cpp:62
+  default:
+    llvm_unreachable("Invalid optimization level!");
+  }
----------------
lxfind wrote:
> So what happens if the compiler is passed with "-Os" or "-Oz"?
Never mind. That won't affect OptLevel.


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

https://reviews.llvm.org/D87596



More information about the llvm-commits mailing list