[PATCH] D92611: [LTO][Legacy] Decouple option parsing from LTOCodeGenerator

wael yehia via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 19 13:29:18 PDT 2021


w2yehia added inline comments.


================
Comment at: llvm/tools/lto/lto.cpp:466
 
+void lto_codegen_debug_options_early(const char *opt) {
+  if (opt) {
----------------
steven_wu wrote:
> If you have to pick an interface, use the new `_array` interface.
> 
> This is also not correct since you skipped `lto_add_attrs` function when you called your version.
> This is also not correct since you skipped lto_add_attrs function when you called your version.

I might be missing something.
`lto_add_attrs` will still run where it usually runs, in `maybeParseOptions`.
Note that I introduced a new boolean, `parsedOptionsEarly`, to indicate whether early parsing happens.
Also, I kept the code in `maybeParseOptions` untouched, so when we call `unwrap(cg)->parseCodeGenDebugOptions()` no parsing would happen if we haven't called `unwrap(cg)->setCodeGenDebugOptions` earlier.

I think some function and variable names could be improved to reflect their intended purpose. 
For example, `LTOCodeGenerator::parseCodeGenDebugOptions` ==> `LTOCodeGenerator::parseSavedCodeGenDebugOptions`.
Also I can add a `LTOCodeGenerator::postOptionParsing` function, to contain code such as the `save-temps` registration.


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

https://reviews.llvm.org/D92611



More information about the llvm-commits mailing list