Thanks Steven.
>A quick feedback will be that (1) is not an option. libLTO APIs need
>to be stable and existing APIs cannot be changed.
Fair point.
>I am curious about your motivation for the change. If you have access
>to `InitTargetOptionsFromCodeGenFlags`, then you don't need libLTO
>interface at all since you are building again LLVM and you are free
>to call any underlying API you want.
InitTargetOptionsFromCodeGenFlags is used in the implementation of libLTO, and
the implementation (not the interface) has access to llvm.
Does that clarify things?
>In libLTO, `parseCodeGenDebugOptions` is a debug function as name
>suggested. People should not rely on that in production. Instead, new
>APIs should be created for the underlying setting you need.
>
The function comments indicate otherwise:
$ git show d5268ebe1925:llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
...
120 /// Pass options to the driver and optimization passes.
121 ///
122 /// These options are not necessarily for debugging purpose (the
function
123 /// name is misleading). This function should be called before
124 /// LTOCodeGenerator::compilexxx(), and
125 /// LTOCodeGenerator::writeMergedModules().
126 void setCodeGenDebugOptions(ArrayRef<StringRef> Opts);
127
128 /// Parse the options set in setCodeGenDebugOptions.
129 ///
130 /// Like \a setCodeGenDebugOptions(), this must be called before
131 /// LTOCodeGenerator::compilexxx() and
132 /// LTOCodeGenerator::writeMergedModules().
133 void parseCodeGenDebugOptions();
> I don't see reasons why you need to parse CodeGen flags before creating
code
> generator.
As I mention in my email:
1. the construction of `llvm::TargetOptions` relies on having parsed the
options
2. the construction of LTOModule relies on the TargetOptions,
3. the LTOCodeGenerator does not rely on TargetOptions, but the factory method
`createCodeGen` which is used in libLTO to construct the opaque type
lto_code_gen_t (which is just a pointer to a LTOCodeGenerator), creates a the
LTOCodeGenerator and then sets the target options:
$ git show d5268ebe1925:llvm/tools/lto/lto.cpp
...
363 static lto_code_gen_t createCodeGen(bool InLocalContext) {
364 lto_initialize();
365
366 TargetOptions Options =
codegen::InitTargetOptionsFromCodeGenFlags(Triple());
367
368 LibLTOCodeGenerator *CodeGen 369 InLocalContext ? new
LibLTOCodeGenerator(std::make_unique<LLVMContext>())
370 : new LibLTOCodeGenerator();
371 CodeGen->setTargetOptions(Options);
372 return wrap(CodeGen);
373 }
374
375 lto_code_gen_t lto_codegen_create(void) {
376 return createCodeGen(/* InLocalContext */ false);
377 }
I could move the creation of TargetOptions and
CodeGen->setTargetOptions(Options) into:
435 static void maybeParseOptions(lto_code_gen_t cg) {
436 if (!parsedOptions) {
437 unwrap(cg)->parseCodeGenDebugOptions();
438 lto_add_attrs(cg);
439 parsedOptions = true;
440 }
441 }
but maybeParseOptions is only called from compile and optimize functions (i.e.
after we've read in all the LTO modules), so LTOModules would have
inaccurate TargetOptions (because command line options have not been parsed
yet).
Wael
-----Steven Wu <stevenwu at apple.com> wrote: -----
>To: Wael Yehia <wyehia at ca.ibm.com>
>From: Steven Wu <stevenwu at apple.com>
>Date: 12/03/2020 06:11PM
>Cc: llvm-dev at lists.llvm.org, joker.eph at gmail.com
>Subject: [EXTERNAL] Re: libLTO Codegen options issue
>
>A quick feedback will be that (1) is not an option. libLTO APIs need
>to be stable and existing APIs cannot be changed.
>
>I am curious about your motivation for the change. If you have access
>to `InitTargetOptionsFromCodeGenFlags`, then you don't need libLTO
>interface at all since you are building again LLVM and you are free
>to call any underlying API you want.
>
>In libLTO, `parseCodeGenDebugOptions` is a debug function as name
>suggested. People should not rely on that in production. Instead, new
>APIs should be created for the underlying setting you need. I don't
>see reasons why you need to parse CodeGen flags before creating code
>generator.
>
>Steven
>
>> On Dec 3, 2020, at 2:57 PM, Wael Yehia <wyehia at ca.ibm.com>
wrote:
>>
>> Hi,
>> We're trying to use libLTO, and noticed an issue (more of a timing
>problem) with option processing where TargetOptions is created before
>cl::ParseCommandLineOptions is invoked.
>> Right now, the only place where
>> ParseCommandLineOptions is called is in
>LTOCodeGenerator::parseCodeGenDebugOptions,
>> which is only called from maybeParseOptions,
>> which is called after TargetOptions have been created.
>> We construct TargetOptions (by calling
>InitTargetOptionsFromCodeGenFlags) first and then use it to we create
>modules or the codegen object.
>>
>> Assuming this is in fact a problem, one way to fix this is to
>decouple parseCodeGenDebugOptions from LTOCodeGenerator, so that it
>can be called before we create the LTOCodeGenerator.
>>
>> Now we can either
>> (1) modify the signature of the lto_codegen_debug_options and
>lto_codegen_debug_options_array API functions
>> or (2) add new API functions.
>>
>> I prefer (1) because as it is now, the API is broken.
>> I uploaded a patch here
>INVALID URI REMOVED
>_D92611&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=z6IPFss9EigepE3DNR_kfA15n
>rHJhAw6dnvEXz_GHvU&m=nxuUhu_XwX2i7vXVLKxoBKtX2GQDqBVZvkN7o0m9ltE&s=Gx
>m0FzQUCAbsP-vv5nMMZCURqJje6Ft2T_t1PEmnnbY&e=
>>
>> Any feedback is appreciated. Thank you
>>
>> Wael
>>
>
>