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

[wael yehia via Phabricator via llvm-commits]

w2yehia added inline comments.


================
Comment at: llvm/tools/lto/lto.cpp:426
+  if (optionParsingState != OptParsingState::Done) {
+    // Parse options if any were set by the lto_codegen_debug_options* function.
     unwrap(cg)->parseCodeGenDebugOptions();
----------------
steven_wu wrote:
> Shouldn't this be:
> ```
> if (optionParsingState ==  OptParsingState::NotParsed)
>   unwrap(cg)->parseCodeGenDebugOptions();
> 
> if (optionParsingState !=  OptParsingState::Done)
>     lto_add_attrs(cg);
> 
> optionParsingState = OptParsingState::Done;
> ```
I considered that... the only disadvantage with guarding `unwrap(cg)->parseCodeGenDebugOptions();` is that now it's not going to run when early option parsing occurs and we would need an additional function in LTOCodeGenerator to contain code that we want to execute **after** debug option parsing has occurred. For example the code to register save-temps (that D98183 is planning to add).

Other than that I think your suggestion is abit more intuitive when reading the code.
I'll let you decide which way we go.
Thanks


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

https://reviews.llvm.org/D92611