[llvm-commits] FW: [PATCH] Fix to pass options from Gold plugin to LTO codegen

[Nick Lewycky]

Viktor Kutuzov wrote:
> Hello Nick,
> Thanks for the review.
> 
> Please find the updated patch attached.
> 
> I didn't remove the lto_codegen_debug_options yet, just marked it as 
> deprecated.
> It is actually used in Ada, in files
> 
> bindings/ada/llvm/llvm_linktimeoptimizer_wrap.cxx
> bindings/ada/llvm/llvm_link_time_optimizer-binding.ads

Bah, then it's too late. We released 2.6 with this API which means that 
it's fixed in stone forever. All of the C API is like that, we will 
never break backwards-compatibility on the C API.

Well, given that, why not just call lto_codegen_debug_options? The 
comment makes it clear that it's "used to pass extra options to the code 
generator" and that's just what we're doing.

> Edward, is this fine to use lto_codegen_set_options there instead?
> 
>> And that would be even better if it took an argc+argv pair
> 
> Gold plug-in gets all arguments from gold one by one.
> It is not a big dial to construct argc+argv pair there and pass it down 
> to the LTO, but LTO internally keeps a vector of char*, so, argc+argv 
> doesn't make much sense unless we want to replace that vector with 
> argc+argv pair as well and to change setCodeGenOptions behavior to set 
> options instead of add them.

Sure, given that lto_codegen_debug_options takes one option at a time 
and we're wedded to this API then we may as well use it.

Please send another patch without the new "lto_codegen_set_options" 
function by calling the "lto_codegen_debug_options" function instead. 
The changes to gold-plugin.cpp look great to me!

Nick

> If this is fine with everyone, I can prepare this patch, but would 
> prefer to make it separately from this one since it would change the API 
> and will break a backward compatibility.
> What do you think?
> 
> Thanks,
> Viktor
> 
> ----- Original Message ----- From: "Nick Lewycky" <nicholas at mxc.ca>
> To: "Viktor Kutuzov" <vkutuzov at accesssoftek.com>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
> Sent: Friday, October 23, 2009 5:56 PM
> Subject: Re: [llvm-commits] FW: [PATCH] Fix to pass options from Gold 
> plugin to LTO codegen
> 
> 
> Viktor Kutuzov wrote:
>> Hello everyone,
>>
>> Please find the patch attached.
>> This should fix the issue when gold plugin doesn't pass the  
>> -plugin-opt options down to the LTO codegen.
>> See the thread "[LLVMdev] getting gold plugin to work?" for more details.
>>
>> This is the second try.
>> It looks like the first one was bounced, not sure why.
>> Sorry if anyone will get it twise.
> 
> Hi Viktor, this looks like the same patch I already reviewed here:
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20091019/089467.html 
> 
> 
> To reiterate:
> "What's the difference between setCodeGenDebugOptions and
> setCodeGenOption? It looks like they should be merged into one then
> exposed by lto_codegen_set_option. And that would be even better if it
> took an argc+argv pair, even if that's a little harder to construct from
> the gold plugin, many option parsing packages work by removing the
> options they recognize from argc+argv and leaving the rest in place."
> 
> Nick
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits