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

[Robinson, Paul via llvm-commits]

> -----Original Message-----
> From: mehdi.amini at apple.com [mailto:mehdi.amini at apple.com]
> Sent: Wednesday, April 13, 2016 10:17 AM
> To: Robinson, Paul
> Cc: Rafael Espíndola;
> reviews+D19015+public+3b72f3e9e30a204c at reviews.llvm.org; Guest, Thomas
> (SCEE); Peter Collingbourne; llvm-commits
> Subject: Re: [PATCH] D19015: [LTO] ensure lto_codegen_debug_options() are
> passed on to LLVM
> 
> 
> > On Apr 13, 2016, at 10:14 AM, Robinson, Paul
> <Paul_Robinson at playstation.sony.com> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: Rafael Espíndola [mailto:rafael.espindola at gmail.com]
> >> Sent: Wednesday, April 13, 2016 8:43 AM
> >> To: reviews+D19015+public+3b72f3e9e30a204c at reviews.llvm.org
> >> Cc: Guest, Thomas (SCEE); Peter Collingbourne; Robinson, Paul; llvm-
> >> commits; Tim Amini Golling
> >> Subject: Re: [PATCH] D19015: [LTO] ensure lto_codegen_debug_options()
> are
> >> passed on to LLVM
> >>
> >> On 13 April 2016 at 11:38, Thomas Guest <thomas_guest at sn.scee.net>
> wrote:
> >>> thomasguest added a comment.
> >>>
> >>> In http://reviews.llvm.org/D19015#399821, @probinson wrote:
> >>>
> >>>> Let me point out that the gold plugin works exactly this way (pass in
> >> cl options as a string).
> >>>> It might not be ideal but it's what we have today.
> >>>> LLVM-as-library doesn't really have another way to conjure up
> >> TargetOptions (I *think* clang creates one directly, sorry can't check
> >> right now, but that seems even more fragile).
> >>>
> >>>
> >>> Thanks for the information @probinson
> >>> I was imagining the way to configure codegen via the C API would be to
> >> add a new function: lto_codegen_configure(lto_codegen_t cg, char **
> argv,
> >> int argc);
> >>
> >> No, you should not expose argv for anything other than debug.
> >>
> >> As it was pointed out, if you want to programmatically  set an option,
> >> you should do something like what clang does.
> >
> > 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.
> 
> I'd argue that this is the ok as an API for LTOCodeGenerator (C++ API,
> tightly coupled, no stability), not for the C API of libLTO.dylib. Are we
> OK on this distinction?

Yes, it's consistent with the "C++ API instability guarantee" :-)
and that's exactly what makes it unsuitable for libLTO.
--paulr

> 
> --
> Mehdi
> 
> 
> > --paulr
> >
> >>
> >> Cheers,
> >> Rafael