<HTML><body>> -----Original Message-----<BR>
> From: Robinson, Paul [mailto:Paul_Robinson@playstation.sony.com]<BR>
> Sent: 13 April 2016 22:19<BR>
> To: mehdi.amini@apple.com; Rafael Espíndola <rafael.espindola@gmail.com><BR>
> Cc: reviews+D19015+public+3b72f3e9e30a204c@reviews.llvm.org; Guest,<BR>
> Thomas <Thomas_Guest@sn.scee.net>; Peter Collingbourne<BR>
> <peter@pcc.me.uk>; llvm-commits <llvm-commits@lists.llvm.org><BR>
> Subject: RE: [PATCH] D19015: [LTO] ensure lto_codegen_debug_options() are<BR>
> passed on to LLVM<BR>
> <BR>
> > > On Apr 13, 2016, at 1:48 PM, Rafael Espíndola<BR>
> > <rafael.espindola@gmail.com> wrote:<BR>
> > ><BR>
> > > On 13 April 2016 at 16:40, Mehdi Amini <mehdi.amini@apple.com> wrote:<BR>
> > >><BR>
> > >>> On Apr 13, 2016, at 1:29 PM, Rafael Espíndola<BR>
> > <rafael.espindola@gmail.com> wrote:<BR>
> > >>><BR>
> > >>>> What clang does is instantiate llvm::TargetOptions and fill in<BR>
> > >>>> the<BR>
> > fields<BR>
> > >>>> directly. That is a *bad API* because it too closely ties the<BR>
> > >>>> caller<BR>
> > and<BR>
> > >>>> callee to lock-step revisions. Clang statically links LLVM so<BR>
> > >>>> there<BR>
> > is<BR>
> > >>>> no versioning problem, but it is definitely not the right library<BR>
> > API.<BR>
> > >>>> --paulr<BR>
> > >>><BR>
> > >>> To solve that you need to add a new C only enum or struct. In the<BR>
> > >>> same way we have lto_codegen_model, we could have<BR>
> lto_codegen_options.<BR>
> > >><BR>
> > >> To be clear, by string-based API, I meant key-value not command<BR>
> > >> line to<BR>
> > parse. Something like:<BR>
> > >><BR>
> > >> set_option("function-section", "1');<BR>
> > >><BR>
> > >> With a giant:<BR>
> > >><BR>
> > >> if(option == "function-section")<BR>
> > >> ...<BR>
> > >> else if(option == ...)<BR>
> > >><BR>
> > >><BR>
> > >> As I said I'm not fond of it (the enum seems cleaner).<BR>
> > ><BR>
> > > If the string is completely local to the C API and unrelated to to<BR>
> > > what the rest of LLVM does, I am in agreement. It is just ugly.<BR>
> > ><BR>
> > > If it is exposing an internal name like a command line option, we<BR>
> > > should not do it. Regardless of how the option is parsed. The list<BR>
> > > and names of cl::opt are not stable, which conflicts with the intent<BR>
> > > of the C api.<BR>
> ><BR>
> > I agree 100% :)<BR>
> <BR>
> Same here. Now Thomas can get on with his new API proposal. :-) --paulr<BR>
<BR>
OK, thanks for the input. I understand:<BR>
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<BR>
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<BR>
<BR>
<br /><br />
<div class="WordSection1"><p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial",sans-serif"><br />**********************************************************************<br />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 </span><a href="mailto:postmaster@scee.net" target="_blank"><span style="font-size:10.0pt;font-family:"Arial",sans-serif">postmaster@scee.net</span></a><span style="font-size:10.0pt;font-family:"Arial",sans-serif"><br />This footnote also confirms that this email message has been checked for all known viruses.<br />Sony Computer Entertainment Europe Limited<br />Registered Office: 10 Great Marlborough Street, London W1F 7LP, United Kingdom<br />Registered in England: 3277793<br />**********************************************************************<br /></span><span style="font-size:18.0pt;font-family:Webdings;color:green"><br />P</span><b><i><span style="font-size:10.0pt;font-family:"Arial",sans-serif;color:green"> Please consider the environment before printing this e-mail</span></i></b> </p></div></body></HTML>