[PATCH] Add an -mattr option to the gold plugin to support subtarget features in LTO

[Nick Lewycky]

Nick Kledzik wrote:
>
> On Mar 25, 2014, at 12:06 AM, Nick Lewycky<nicholas at mxc.ca>  wrote:
>
>> Nick Kledzik wrote:
>>>
>>> On Mar 24, 2014, at 4:27 PM, Tom Roeder<tmroeder at google.com>   wrote:
>>>> The small patch attached here adds support for an -mattr option to the
>>>> gold plugin and to llvm-lto. This allows the caller to specify details
>>>> of the subtarget architecture, like +aes, or +ssse3 on x86.  Note that
>>>> this required a change to the include/llvm-c/lto.h interface: it adds
>>>> a function lto_codegen_set_attr and it increments the version of the
>>>> interface.
>>>>
>>>> Please let me know what you think.
>>>
>>> The LTO API already has:
>>>     lto_codegen_debug_option(lto_codegen_t, const char *);
>>>
>>> Could you support -mattr options using the existing API like?:
>>>      lto_codegen_debug_option(cg, “-mattr=+sse3”);
>>
>> Absolutely not. That API has the word "debug" in its name because it is strictly reserved for debugging libLTO. That means that it is not part of libLTO's public interface.
> It is in the public header<llvm-c/lto.h>.  How can it not be part of the public interface?
>
>
>> The reason is that libLTO has a strong ABI requirement. Once we have a user using libLTO, we can never ever break them at the ABI level. The intention is that this exposed interface is something small that we can audit, and that we can change LLVM's innards without disturbing libLTO.
>>
>> Once you allow passing arbitrary flags across, every single flag everywhere in every llvm library becomes part of the permanent ABI that can never be changed.
> That boat has already sailed.

No. You may have shipped a product you thought was ABI stable, but you 
are mistaken.

   The darwin linker accepts “-mllvm foo” and calls 
lto_codegen_debug_option(xx, “foo”).

Realize that flags are currently whole-process globals. LLVM is a 
library, and there is no place for such globals (consider a program that 
uses a graphics library and an audio library, both libraries are 
accelerated using llvm under the hood. They can not both set these 
globals). The direction we're headed is one where flags in the llvm 
libraries get transformed into APIs and non-global state objects while 
the flags themselves get shunted outwards towards the tools. Ultimately 
"-mattr" will be a flag in tools/llc/llc.cpp not inside the llvm library.

What then?

If you want to maintain backwards compatibility, I suggest you find a 
set of "mllvm" flags you want to continue to support, then add a new 
string-based API for them to libLTO and have libLTO parse those flags 
and set llvm's internal state.

And no, you can't use lto_codegen_debug_option for that purpose. That 
function is for passing arbitrary flags to debug libLTO with. What this 
means is that you've already bought into ABI instability and the linker 
you've already shipped will not be compatible with some future version 
of libLTO.

  It has been used with things like -mcpu=blah to work around that 
information not being recorded in the bitcode files.
>
> I understand that a string interface by-passes any sort of static analysis.

I have no problem with a string interface. I do have a problem with 
putting the entirety of llvm libraries under ABI lock.

> Isn’t the long term goal that info like +sse3 be recorded in bit code files so that these options are not needed at link time?  If so, then the proposed API is a short term work around.  My point is that we already have a hack to pass arbitrary option strings.  Why have two?

If the plan was to bake arbitrary strings into bitcode and call 
cl::ParseCommandLineOptions then we need a new plan. However, I see no 
problem with baking into the bitcode file the same strings that the 
backend .td files use ("sse3").

I don't even mind if you want to make the string "mattr=+sse3", the only 
requirement is that it needs to be parsed by libLTO itself or something 
that's aware of the ABI stability guarantee.

Nick