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

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 13 13:26:51 PDT 2016


On 13 April 2016 at 13:14, Mehdi AMINI <mehdi.amini at apple.com> wrote:
> joker.eph added a comment.
>
> In http://reviews.llvm.org/D19015#400002, @probinson wrote:
>
>> In http://reviews.llvm.org/D19015#400001, @joker.eph wrote:
>>
>> > > 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);
>> >
>> >
>> > You got me wrong: the problem is not with the name of the API, it is with the fact that we don't want *any command line* argument, i.e. we should never call `cl::ParseCommandLineOptions` as part of a normal flow (i.e. other than debug).
>>
>>
>> Passing in string options is not the objection that I understood (from previous this-is-a-library discussion).  The problem is that cl::opt parses them *using static data* and that's not good for a library.  Parsing string options to an API that then implemented the parsing without using static data would be fine.
>
>
> You're definitively right. The objection this is about `cl::opt`, which is what I referred to by "command line options" (the argv/argc in the proposed prototype lead me to believe that the intention was to call `cl::ParseCommandLineOptions()`).
> I'm not fond of string based APIs but I can live with them :)

Please don't. Even if we can solve the static constructor problem we
really should not be parsing command line options.

Given the desire for stability, we have to do what every other C api
does: define its own stable enum of options and convert that to what
llvm uses internally.

Cheers,
Rafael


More information about the llvm-commits mailing list