<div dir="ltr">On 12 August 2013 13:29, Shuxin Yang <span dir="ltr"><<a href="mailto:shuxin.llvm@gmail.com" target="_blank">shuxin.llvm@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><div><div class="h5">
<div>On 8/12/13 1:03 PM, Nick Lewycky wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">On 12 August 2013 12:22, Shuxin Yang <span dir="ltr"><<a href="mailto:shuxin.llvm@gmail.com" target="_blank">shuxin.llvm@gmail.com</a>></span>
wrote:<br>
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div>On 8/12/13 12:16 PM, Nick Lewycky wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Shuxin Yang wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Author: shuxin_yang<br>
Date: Mon Aug 12 13:29:43 2013<br>
New Revision: 188188<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=188188&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=188188&view=rev</a><br>
Log:<br>
Misc enhancements to LTO:<br>
<br>
1. Add some helper classes for partitions. They
are designed in a<br>
way such that the top-level LTO driver will
not see much difference<br>
with or without partitioning.<br>
<br>
2. Introduce work-dir. Now all intermediate files
generated during<br>
LTO phases will be saved under work-dir. User
can specify the workdir<br>
via -lto-workdir=/path/to/dir. By default the
work-dir will be<br>
erased before linker exit. To keep the
workdir, do -lto-keep, or -lto-keep=1.<br>
<br>
TODO: Erase the workdir, if the linker exit
prematurely.<br>
We are currently not able to remove directory
on signal. The support<br>
routines simply ignore directory.<br>
<br>
3. Add one new API lto_codegen_get_files_need_remove().<br>
Linker and LTO plugin will communicate via
this API about which files<br>
(including directories) need to removed before
linker exit.<br>
</blockquote>
<br>
Please revert. Adding new flags to libLTO is the wrong
direction (in spite of the ones that exist -- consider
those grandfathered in). <br>
</blockquote>
</div>
It dose not make sense. Without flags, how do you manage
to triage the correctness and performance problem?<br>
</blockquote>
<div><br>
</div>
<div>Something else has flags, </div>
</div>
</div>
</div>
</blockquote>
<br></div></div>
What are "something else"? As far as I know, there are only two
fall into this category:<br>
<br>
- Apple linker, and<br>
- GNU gold. <br>
<br>
The former communicate with the libLTO directly with these APIs,
while GNU gold communicate <br>
with the libLTO via, which I called adapter. <br></div></blockquote><div><br></div><div>You committed the patch, you tell me. What did this patch intend to change the behaviour of?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> Directly calling these APIs is really bad idea. I manage to
convince the black-belt guru Nick @ Apple to <br>
not directly calling these APIs in the new ld design. The linker's
should be LTO oblivious, the linker <br>
should expose symbol-related interface instead of LTO-control
interfaces. <br></div></blockquote><div><br></div><div>Directly calling which APIs?</div><div><br></div><div>What do you mean by "LTO oblivious"? Oblivious towards whether optimization is being performed?</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><div class="im">
>which in turn drives libLTO through the API.<br>
<br></div>
Depending on the what kind of info "something" else need to drive
the libLTO. <br>
In general it is very bad idea, if "something else" need
micro-management.</div></blockquote><div><br></div><div>libLTO is part of the linker that uses it. Having a default setting with the ability to override it is a sensible convenience for users of libLTO.</div><div><br>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">Take Apple ld as example, if I want to change LTO in a way such
that I don't want to load all module, <br>
I just want to load summary info. Current APIs are not sufficient. I
have to modify the API, or add new APIs<br>
to that matter, in the mean time, I need release the new ld to the
user in order to accomodate the change. <br>
that is nightmare. </div></blockquote><div><br></div><div>The point of libLTO is to provide an ABI-fixed library, isolating the linker from llvm's internals. 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.</div>
<div><br></div><div>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">Sure, "something else" can control the libLTO, if it want. In my
case, if "something else" want specify <br>
a workdir, then go ahead. Otherwise, the libLTO use default one. Is
there any wrong here?</div></blockquote><div><br></div><div>At a high level that sounds fine to me. The wrong part is using flags to do it.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><div class="im"><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Adding flags to linker instead, I think that is wrong
direction. Linker dose not have data structure which
libLTO dose.</blockquote>
<div><br>
</div>
<div>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?</div>
</div>
</div>
</div>
</blockquote></div>
I actually discuss with Nick @ Apple before. The conclusion is
linker must be LTO oblivious, <br>
it should think in symbol-way, and talk in symbol way (as with GNU
gold). It would otherwise<br>
very very troublesome both for linker and libLTO. <br></div></blockquote><div><br></div><div>And now you're discussing it with me. I also agree that the linker should communicate primarily in symbols and about symbols with libLTO.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">On the other hand, we now have two linkers support LTO. There are
different way to control <br>
the libLTO (even for simple task, like save intermediate files), how
messy?<br>
<br>
I'd like to move all these stuff to libLTO to have a unified
control. <br></div></blockquote><div><br></div><div>I have no problem with a unified control.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><div class="im"><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div>libLTO is intended to be used as a
library, it may not get a chance to parse flags.<br>
</div>
It has to. Prior to my change, linkers (GNU linker and
Apple ld) pass arch to linker, via a function<br>
confusingly called, something like "add.*debug.*options".</blockquote>
<div><br>
</div>
<div>Can't. If we allow this, every flag in every part of
LLVM that libLTO links against is baked into the C ABI
forever.</div>
<div><br>
</div>
<div>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.</div>
</div>
</div>
</div>
</blockquote></div>
addDebugOptions is misnomer. It is also passes essential flags like
-arch=x86. Without such flags, <br>
the LTO dose not even compile. <br></div></blockquote><div><br></div><div>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?</div>
<div><br></div><div>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'm sorry you decided to land three things together in one patch, please remember not to do that in the future.<br>
</div><div><br></div><div>Nick</div><div><br></div><div>* <a href="http://www.youtube.com/watch?v=cNZKUozrBl4&t=1m33s">http://www.youtube.com/watch?v=cNZKUozrBl4&t=1m33s</a></div></div></div></div>