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

[Nick Kledzik]

Tom,

Your patch is fine with me.  It is a reasonable work around until we start encoding all those attributes in bitcode. 

-Nick K.

On Apr 21, 2014, at 10:16 AM, Tom Roeder <tmroeder at google.com> wrote:

> On Thu, Mar 27, 2014 at 2:54 AM, Nick Lewycky <nicholas at mxc.ca> wrote:
>> 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.
>> 
> 
> This patch is currently the only short-term way I know to get these
> attributes down into LTO, and this is necessary for me to get some
> large LTO builds to work. (That is, the only way other than the debug
> interface, which seems to have strong opposition on this list). LTO is
> currently broken for builds that use special attributes to compile
> some files.  Is this patch acceptable for fixing that breakage?
> 
> Thanks,
> 
> Tom