[PATCH] D19015: [LTO] ensure lto_codegen_debug_options() are passed on to LLVM

Robinson, Paul via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 13 14:18:39 PDT 2016


> > On Apr 13, 2016, at 1:48 PM, Rafael EspĂ­ndola
> <rafael.espindola at gmail.com> wrote:
> >
> > On 13 April 2016 at 16:40, Mehdi Amini <mehdi.amini at apple.com> wrote:
> >>
> >>> On Apr 13, 2016, at 1:29 PM, Rafael EspĂ­ndola
> <rafael.espindola at gmail.com> wrote:
> >>>
> >>>> What clang does is instantiate llvm::TargetOptions and fill in the
> fields
> >>>> directly.  That is a *bad API* because it too closely ties the caller
> and
> >>>> callee to lock-step revisions.  Clang statically links LLVM so there
> is
> >>>> no versioning problem, but it is definitely not the right library
> API.
> >>>> --paulr
> >>>
> >>> To solve that you need to add a new C only enum or struct. In the same
> >>> way we have lto_codegen_model, we could have lto_codegen_options.
> >>
> >> To be clear, by string-based API, I meant key-value not command line to
> parse. Something like:
> >>
> >> set_option("function-section", "1');
> >>
> >> With a giant:
> >>
> >> if(option == "function-section")
> >>  ...
> >> else if(option == ...)
> >>
> >>
> >> As I said I'm not fond of it (the enum seems cleaner).
> >
> > If the string is completely local to the C API and unrelated to to
> > what the rest of LLVM does, I am in agreement. It is just ugly.
> >
> > If it is exposing an internal name like a command line option, we
> > should not do it. Regardless of how the option is parsed. The list and
> > names of cl::opt are not stable, which conflicts with the intent of
> > the C api.
> 
> I agree 100% :)

Same here. Now Thomas can get on with his new API proposal. :-)
--paulr
 


More information about the llvm-commits mailing list