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

wael yehia via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 30 12:45:53 PDT 2021


w2yehia added a comment.

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.


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

https://reviews.llvm.org/D92611



More information about the llvm-commits mailing list