[llvm-dev] libLTO Codegen options issue
Steven Wu via llvm-dev
llvm-dev at lists.llvm.org
Thu Dec 3 16:28:00 PST 2020
> On Dec 3, 2020, at 4:14 PM, Wael Yehia <wyehia at ca.ibm.com> wrote:
>
> 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();
We don't officially support any option that has to pick up by cl::opt.
>
>> 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).
Module doesn't have TargetOption. All it carries is the target triple and all the feature flags in the metadata. I would suggest you come with solid example about which cl::opt you are trying to overwrite and let's see if we can make an API out of it.
Steven
>
> Wael
>
> -----Steven Wu <stevenwu at apple.com <mailto:stevenwu at apple.com>> wrote: -----
>
>> To: Wael Yehia <wyehia at ca.ibm.com <mailto:wyehia at ca.ibm.com>>
>> From: Steven Wu <stevenwu at apple.com <mailto:stevenwu at apple.com>>
>> Date: 12/03/2020 06:11PM
>> Cc: llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>, joker.eph at gmail.com <mailto: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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201203/b2efb9a6/attachment.html>
More information about the llvm-dev
mailing list