<div dir="ltr">On 12 August 2013 15:00, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><div><div class="h5">
<div>On 8/12/13 2:32 PM, Nick Lewycky wrote:<br>
</div>
<blockquote type="cite">
<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>
<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>
</div>
</div>
</blockquote></div></div>
As I already told you, the only "something else" I'm aware of are
Apple linker and GNU linker. <br>
<br>
How can change their behavior, they are released as binary. <br>
<br>
I did a new API, which dose not affect Apple ld. Of course, it dose
not change GNU ld either. <br>
However, we need to change gold-plugin (tool/gold/gold-plugin.cpp) a
lit, such that, <br>
<br>
it get a list of files to be removed from libLTO.xxx.<br>
<br>
Prior to my change, the gold-plugin.cpp delete the intermediate
object file as it has only <br>
one intermediate file. It was working ok, but not anymore.<br>
<br>
The linker see no difference. <br><div class="im">
<br>
<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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>
</div>
</div>
</blockquote>
<br></div>
The linker dose not have any functions regarding lto, take a look
of GNU gold API or tool/gold/*.cpp<div class="im"><br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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>
>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. </div>
</div>
</div>
</div>
</blockquote>
<br></div>
No! Absolutely not!</div></blockquote><div><br></div><div>Fair enough. I meant "libLTO is part of the linker that uses it" in the same sense that a networking library is part of the web browser that uses it. The library shouldn't be off deciding to do things of its own accord, it should provide an API that allows something else to accomplish its task.</div>
<div><br></div><div>I don't see this as a very bad idea or as micro-management. In this regard, I don't see a difference between libLTO and other libraries like libPNG or netlib or freetype. (There is a difference in that we want libLTO to be a very high-level interface instead of exposing the details of .bc files, entirely unlike what libPNG does for PNG or freetype does for font files.)</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc 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">
<div>Having a default setting with the ability to override
it is a sensible convenience for users of libLTO.</div>
</div>
</div>
</div>
</blockquote>
<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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. </div>
</div>
</div>
</div>
</blockquote></div>
It is not "fixed", it is changing constantly.</div></blockquote><div><br></div><div>The only reason libLTO exists at all is to give the linker something to link against which will have a fixed ABI. Same with "libclang" on the clang side.</div>
<div><br></div><div>Thus far it's LLVM policy that the *whole and entire* C API is ABI-fixed forever, and I've argued a few times on the mailing list that this can't be right, and that only libLTO and libClang ought to be ABI locked.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"> E.g. the APIs used to
take for granted the libLTO return only one objects, <br>
now I need to return multiple.<br></div></blockquote><div><br></div><div>Yes, and that's a problem. Not your problem really, except to the degree that you inherited it. The existing APIs in libLTO weren't nearly forwards-compatible enough, and now we're in trouble.</div>
<div><br></div><div>Unless we figure out something clever, we may have to add a whole new set of functions to libLTO, and not deprecate the existing ones (at least, not unless we get consensus on llvm-dev that it's okay to break our previous ABI promise).</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc 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">
<div>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>
</div>
</div>
</div>
</blockquote></div>
then how to change the behavior for say, debugging purpose. <br></div></blockquote><div><br></div><div>Debugging is special. In theory, you don't even need to commit to upstream for debugging, but it's fine to add features that are helpful. We have that sort of thin all over llvm. libLTO has addDebugOptions to permit this sort of debugging usage, but it shouldn't be used in the non-debugging case.</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc 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 bgcolor="#FFFFFF" text="#000000"><div><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>
<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.</div>
</div>
</div>
</div>
</blockquote>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>* Honestly, I looked and couldn't find a -arch flag
that libLTO would interpret. How sure are you about this?</div>
</div>
</div>
</div>
</blockquote></div>
Perhaps not -arch flags.<br>
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. <br><div class="im">
<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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.</div>
</div>
</div>
</div>
</blockquote></div>
I have no alternative. If I introduce a workdir, I need to have to
way to inform linker-plugin to get rid of way. <br>
This is another example why those API sucks.<br></div></blockquote><div><br></div><div>You don't have the source code to the linker?</div><div><br></div><div>Let's focus on this, it sounds like this is the key problem. What's wrong with modifying the linker if you want to change the behaviour of your linker?</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc 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">
<div> I'm sorry you decided to land three things together in
one patch, please remember not to do that in the future.<br>
</div>
<br>
</div>
</div>
</div>
</blockquote></div>
Ok, tell me how to create temp workding directory right. How to save
temp files right both for gold and Apple ld. <br></div></blockquote><div><br></div><div>*Why*? Are you implementing this as a linker feature you intend to ship in the real linker? Or is this to debug the innards of libLTO?</div>
<div><br></div><div>The only case I *am* okay with flags is when we all agree they're flags for debugging the internals of libLTO, and that we don't ship products that rely on them. I explicitly called that out. If the only purpose of these was to implement debugging features, then I'm sorry for the miscommunication!</div>
<div><br></div><div>If the intention is to let libLTO run on machines that don't have /tmp (this is what I thought), we should give libLTO an API that lets the linker decide where the files go. Maybe it wants to do smart things like putting it in a directory with the right permissions, or which is scheduled for cleanup in the event of a crash.</div>
<div><br></div><div>If the intention is to implement something like -r, or to implement some new feature like -shared but which emits a shared-bitcode file instead of a shared-object, then I don't think that controlling the working directory then pulling out intermediate files is the right design anyways.</div>
<div><br></div><div>Or are you doing something else?</div><div><br></div><div>Nick</div></div></div></div>