[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