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

[Guest, Thomas via llvm-commits]

> -----Original Message-----
> From: Robinson, Paul [mailto:Paul_Robinson at playstation.sony.com]
> Sent: 13 April 2016 22:19
> To: mehdi.amini at apple.com; Rafael Espíndola <rafael.espindola at gmail.com>
> Cc: reviews+D19015+public+3b72f3e9e30a204c at reviews.llvm.org; Guest,
> Thomas <Thomas_Guest at sn.scee.net>; Peter Collingbourne
> <peter at pcc.me.uk>; llvm-commits <llvm-commits at lists.llvm.org>
> Subject: RE: [PATCH] D19015: [LTO] ensure lto_codegen_debug_options() are
> passed on to LLVM
> 
> > > 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

OK, thanks for the input. I understand:
1) the preferred way to extend the C API to configure codegen is to add some C types which reflect the various available codegen options, and allow clients to pass these into a new lto_codegen_set_options() API function
2) the result of this must be applied only to the codegen object passed into lto_codegen_set_options(), and not to some global cl::opts
 

**********************************************************************
This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you have received this email in error please notify postmaster at scee.net
This footnote also confirms that this email message has been checked for all known viruses.
Sony Computer Entertainment Europe Limited
Registered Office: 10 Great Marlborough Street, London W1F 7LP, United Kingdom
Registered in England: 3277793
**********************************************************************

Please consider the environment before printing this e-mail
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160414/3ca088f5/attachment.html>