[llvm-dev] libLTO Codegen options issue
Mehdi AMINI via llvm-dev
llvm-dev at lists.llvm.org
Thu Dec 3 16:23:10 PST 2020
On Thu, 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();
>
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.
--
Mehdi
>
> > 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
> >>
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201203/55b06491/attachment.html>
More information about the llvm-dev
mailing list