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

[Steven Wu via Phabricator via llvm-commits]

steven_wu added a comment.

In D92611#2659603 <https://reviews.llvm.org/D92611#2659603>, @w2yehia wrote:

> In D92611#2659437 <https://reviews.llvm.org/D92611#2659437>, @steven_wu wrote:
>
>> In D92611#2659079 <https://reviews.llvm.org/D92611#2659079>, @w2yehia wrote:
>>
>>> @steven_wu thanks for your review. Is this good to be merged then?
>>
>> Can you just revert to use `parsedOptions` as boolean? The state you used right now doesn't add anything. You can just leave `lto_set_debug_options` not guarded right?
>
> I consider the two APIs `lto_set_debug_options` and `lto_codegen_debug_options` to be mutually exclusive, so I need to keep track whether `lto_set_debug_options` was called.
> Before creating the enum state, I had two booleans `parsedOptions` and `parsedOptionsEarly` where `parsedOptionsEarly` is a boolean I added to tell that early options has been set (i.e. `lto_set_debug_options` called).
> However, I cannot set `parsedOptions` when my new API function is called because I want `maybeParseOptions` to still do it's thing. So I needed that second boolean.
>
> So I then created the state variable in place of the two booleans (cleaner in my opinion and avoids having to check combinations of the two booleans) to tell which state we're in:
>
>   NotParsed, // Initial state. before either lto_set_debug_options or maybeParseOptions is called.
>   Early,     // After lto_set_debug_options is called. This state will be skipped if user calls lto_codegen_debug_options.
>   Done       // After maybeParseOptions is called (i.e. unwrap(cg)->parseCodeGenDebugOptions() and lto_add_attrs(cg) have executed)
>
> The state can either change `NotParsed -> Done`  or `NotParsed -> Early -> Done` depending on which API was used to set debug options.

Is there a problem if both APIs are called (providing they don't conflict with each other or duplicate arguments?)


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

https://reviews.llvm.org/D92611