[llvm-dev] libLTO Codegen options issue
Wael Yehia via llvm-dev
llvm-dev at lists.llvm.org
Fri Dec 4 06:17:52 PST 2020
Hi Steven and Mehdi,
>Module doesn't have TargetOption. All it carries is the target triple
>and all the feature flags in the metadata.
I was talking about LTOModule, whose factory method (LTOModule::makeLTOModule) takes a `TargetOptions` object.
It uses it to create a TargetMachine.
> 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.
please see my reasoning in my next paragraph.
> In general, I see any cl::opt as a debugging tool. Clients should use
> programmatic API and not rely on string-based CL opt to control what's
> happening behind the APIs.
I see, makes sense.
But what if the cl::opt, that is used for debugging only, is not respected when you're actually trying to use it during debugging?
This is the issue here, the mechanism by which you can pass "debugging" options to libLTO has a flaw where as a result, some cl::opt objects are read too soon, before option parsing has occurred.
To be specific, we were adding a cl::opt option to control the value of a field in a `TargetOptions` object when the object is created via InitTargetOptionsFromCodeGenFlags. This is nothing new, as there are already a bunch of options (for example -march, -mcpu, -relocation-model) in CommandFlags.cpp that have an effect on TargetOptions objects created via InitTargetOptionsFromCodeGenFlags (which libLTO currently calls)
Surely, we don't wanna add a new function to the libLTO API for every option in CommandFlag.cpp?
Also please note, that we as the client of libLTO are an intermediary between the real user and LTO. And the real user when debugging his application will try passing options to the LTO component that he sees are listed with --help, and so options defined in CommandFlag.cpp won't behave as expected.
Thanks everyone.
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 07:28PM
>Cc: llvm-dev at lists.llvm.org, joker.eph at gmail.com
>Subject: [EXTERNAL] Re: libLTO Codegen options issue
>
> On Dec 3, 2020, at 4:14 PM, Wael Yehia
> wrote:...
>
>
>
>
>
> This Message Is From an
>External Sender
>
> This message came from outside your organization.
>
>
>
>
>
>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> 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
>
More information about the llvm-dev
mailing list