[llvm] r188188 - Misc enhancements to LTO:

[Shuxin Yang]

On 8/12/13 2:32 PM, Nick Lewycky wrote:
> On 12 August 2013 13:29, Shuxin Yang <shuxin.llvm at gmail.com 
> <mailto:shuxin.llvm at gmail.com>> wrote:
>
>     On 8/12/13 1:03 PM, Nick Lewycky wrote:
>>     On 12 August 2013 12:22, Shuxin Yang <shuxin.llvm at gmail.com
>>     <mailto:shuxin.llvm at gmail.com>> wrote:
>>
>>         On 8/12/13 12:16 PM, Nick Lewycky wrote:
>>
>>             Shuxin Yang wrote:
>>
>>                 Author: shuxin_yang
>>                 Date: Mon Aug 12 13:29:43 2013
>>                 New Revision: 188188
>>
>>                 URL:
>>                 http://llvm.org/viewvc/llvm-project?rev=188188&view=rev
>>                 Log:
>>                 Misc enhancements to LTO:
>>
>>                    1. Add some helper classes for partitions. They
>>                 are designed in a
>>                       way such that the top-level LTO driver will not
>>                 see much difference
>>                       with or without partitioning.
>>
>>                    2. Introduce work-dir. Now all intermediate files
>>                 generated during
>>                       LTO phases will be saved under work-dir. User
>>                 can specify the workdir
>>                       via -lto-workdir=/path/to/dir. By default the
>>                 work-dir will be
>>                       erased before linker exit. To keep the workdir,
>>                 do -lto-keep, or -lto-keep=1.
>>
>>                      TODO: Erase the workdir, if the linker exit
>>                 prematurely.
>>                        We are currently not able to remove directory
>>                 on signal. The support
>>                        routines simply ignore directory.
>>
>>                    3. Add one new API
>>                 lto_codegen_get_files_need_remove().
>>                       Linker and LTO plugin will communicate via this
>>                 API about which files
>>                      (including directories) need to removed before
>>                 linker exit.
>>
>>
>>             Please revert. Adding new flags to libLTO is the wrong
>>             direction (in spite of the ones that exist -- consider
>>             those grandfathered in).
>>
>>         It dose not make sense. Without flags, how do you manage to
>>         triage the correctness and performance problem?
>>
>>
>>     Something else has flags,
>
>     What are "something else"?  As far as I know, there are only two
>     fall into this category:
>
>        - Apple linker, and
>        - GNU gold.
>
>       The former communicate with the libLTO directly with these APIs,
>     while GNU gold communicate
>     with the libLTO via, which I called adapter.
>
>
> You committed the patch, you tell me. What did this patch intend to 
> change the behaviour of?
As I already told you, the only "something else" I'm aware of are Apple 
linker and GNU linker.

How can change their behavior, they are released as binary.

I did a new API, which dose not affect Apple ld. Of course, it dose not 
change GNU ld either.
However, we need to change gold-plugin (tool/gold/gold-plugin.cpp) a 
lit, such that,

    it get a list of files to be removed from libLTO.xxx.

    Prior to my change, the gold-plugin.cpp delete the intermediate 
object file as it has only
one intermediate file. It was working ok, but not anymore.

   The linker see no difference.



>
>        Directly calling these APIs is really bad idea. I manage to
>     convince the black-belt guru Nick @ Apple to
>     not directly calling these APIs in the new ld design. The linker's
>     should be LTO oblivious, the linker
>     should expose symbol-related interface instead of LTO-control
>     interfaces.
>
>
> Directly calling which APIs?
>
> What do you mean by "LTO oblivious"? Oblivious towards whether 
> optimization is being performed?

The linker dose not have any functions regarding lto,  take a look of 
GNU gold API or tool/gold/*.cpp

>
>     >which in turn drives libLTO through the API.
>
>     Depending on the what kind of info "something" else need to drive
>     the libLTO.
>     In general it is very bad idea, if "something else" need
>     micro-management.
>
>
> libLTO is part of the linker that uses it.

No! Absolutely not!
> Having a default setting with the ability to override it is a sensible 
> convenience for users of libLTO.


>
>     Take Apple ld as example,  if I want to change LTO in a way such
>     that I don't want to load all module,
>     I just want to load summary info. Current APIs are not sufficient.
>     I have to modify the API, or add new APIs
>     to that matter, in the mean time, I need release the new ld to the
>     user in order to accomodate the change.
>     that is nightmare.
>
>
> The point of libLTO is to provide an ABI-fixed library, isolating the 
> linker from llvm's internals.
It is not "fixed", it is changing constantly. E.g. the APIs used to take 
for granted the libLTO return only one objects,
now I need to return multiple.

> That in turn leads to a few design decisions. The API is designed to 
> refer to high-level concepts instead of the details of llvm's actual 
> behaviour. Things like module lazy loading or setting the datalayout 
> are excluded from the API. Flags are even more private, surely we 
> should be able to change flags in LLVM's libraries without worrying 
> about breaking linkers.
>
> If the linker needs to do something where it matters how llvm is 
> implemented -- you mention loading summary info, I'll assume you mean 
> lazy-loading the module such that function bodies aren't loaded -- 
> then the linker doesn't use libLTO at all, but uses llvm directly. 
> Conversely, libLTO knows all about llvm and will lazy-load .bc files 
> without being asked to.
>
>     Sure, "something else" can control the libLTO, if it want. In my
>     case, if "something else" want specify
>      a workdir, then go ahead. Otherwise, the libLTO use default one.
>     Is there any wrong here?
>
>
> At a high level that sounds fine to me. The wrong part is using flags 
> to do it.
then how to change the behavior for say, debugging purpose.

>>         Adding flags to linker instead, I think that is wrong
>>         direction. Linker dose not have data structure which libLTO dose.
>>
>>
>>     This is the discussion to have. What things do you need here
>>     which you don't think should be exposed through the API, and yet
>>     you want to be exposed for you?
>     I actually discuss with Nick @ Apple before.  The conclusion is
>     linker must be LTO oblivious,
>     it should think in symbol-way, and talk in symbol way (as with GNU
>     gold). It would otherwise
>      very very troublesome both for linker and libLTO.
>
>
> And now you're discussing it with me. I also agree that the linker 
> should communicate primarily in symbols and about symbols with libLTO.
>
>     On the other hand, we now have two linkers support LTO. There are
>     different way to control
>     the libLTO (even for simple task, like save intermediate files),
>     how messy?
>
>     I'd like to move all these stuff to libLTO to have a unified control.
>
>
> I have no problem with a unified control.
>
>>         libLTO is intended to be used as a library, it may not get a
>>         chance to parse flags.
>>         It has to. Prior to my change, linkers (GNU linker and Apple
>>         ld) pass arch to linker, via a function
>>         confusingly called, something like "add.*debug.*options".
>>
>>
>>     Can't. If we allow this, every flag in every part of LLVM that
>>     libLTO links against is baked into the C ABI forever.
>>
>>     Of course addDebugOptions does allow this, but it's named (and I
>>     thought documented in the comments) such that anybody using it
>>     knows they're using a non-stable non-production debugging API.
>>     Anybody using addDebugOptions for something other than debugging
>>     libLTO is living outside the ABI guarantees.
>     addDebugOptions is misnomer. It is also passes essential flags
>     like -arch=x86.  Without such flags,
>     the LTO dose not even compile.
>
>
> That sounds like a nice bug you've got there! Wouldn't want anything 
> to happen to it. It'd be a shame if breaks before you manage to add a 
> liblto_set_arch() function for it.

> * Honestly, I looked and couldn't find a -arch flag that libLTO would 
> interpret. How sure are you about this?
Perhaps not -arch flags.
But at least some flags are passed this way.  I remember we use this way 
to pass -fast-math before Bill's attribute-stuff is working.


> In case it isn't completely clear, flags are absolutely right out. 
> Either you will revert this patch, or I will revert it for you.
I have no alternative.  If I introduce a workdir, I need to have to way 
to inform linker-plugin to get rid of way.
This is another example why those API sucks.


> I'm sorry you decided to land three things together in one patch, 
> please remember not to do that in the future.
>
Ok, tell me how to create temp workding directory right. How to save 
temp files right both for gold and Apple ld.




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130812/41260297/attachment.html>