<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 22, 2016 at 10:41 AM, Teresa Johnson <span dir="ltr"><<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Fri, Jul 22, 2016 at 10:35 AM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Fri, Jul 22, 2016 at 10:06 AM, Teresa Johnson <span dir="ltr"><<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">tejohnson added a comment.<br>
<span><br>
> I thought one of the issue is that the first link does not drop any, but the second link drops it if the same command line is used.  If the first link does not drop any, is v1.12 still needed?<br>
<br>
<br>
</span>Ah, good point, I had forgotten that the issue there was that we don't want to drop one file (and always include all the rest). (In some cases you can get into trouble if you include all objects from the archives, but not there).<br>
<br>
That being said, since the patch no longer addresses the specific instance in that test case, I think it is a bit too heavy-weight - we know that it is fixed by including that second file in the link, and the test case I added here ensures that the file does include objects from libraries that are needed by the link. So my preference is not to include it here, but if you feel strongly I will add it too.<br></blockquote><div><br></div><div><br></div></span><div>Probably as a follow up patch.  </div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span><br>
<br>
================<br>
Comment at: test/tools/gold/X86/thinlto_emit_linked_objects.ll:18<br>
@@ +17,3 @@<br>
+; RUN:    %t.o \<br>
+; RUN:    --start-lib %t2.o --end-lib<br>
+<br>
----------------<br>
davidxl wrote:<br>
> tejohnson wrote:<br>
> > davidxl wrote:<br>
> > > tejohnson wrote:<br>
> > > > davidxl wrote:<br>
> > > > > Perhaps add test case about real archive case as well.<br>
> > > > For a distributed build the build system needs to extract the constituent objects (otherwise the combined index paths will not point to an object file)<br>
> > > I think this feature is more about two step link (not about a particular distributed build system). It is possible that some other distributed build system uses archive, right?<br>
> > Using archives directly with a distributed build won't work - the combined index module paths need to point to an object file for the distributed backends to import from. The build system needs to extract the objects (e.g. into a temp dir) and pass those.<br>
> ><br>
> > Note that archives passed to a non-distributed build work fine because we load the modules from the offset within the archive passed to the plugin from gold and serve those to the importer.<br>
</span>> how about thin archive?<br>
Gold does not seem to like thin archives (regardless of LTO vs ThinLTO). I get an error (from gold, not the plugin) like:<br>
<br>
error: foo.a: no archive symbol table (run ranlib)<br>
<br>
(even when I run ranlib)<br></blockquote><div><br></div><div><br></div><div><br></div></span><div>ok -- a path not yet supported then.</div><span><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span><br>
================<br>
Comment at: test/tools/gold/X86/thinlto_emit_linked_objects.ll:21<br>
@@ +20,3 @@<br>
+; RUN: cat %t3 | FileCheck %s<br>
+; CHECK: thinlto_emit_linked_objects.ll.tmp.o<br>
+; CHECK: thinlto_emit_linked_objects.ll.tmp2.o<br>
----------------<br>
davidxl wrote:<br>
> tejohnson wrote:<br>
> > davidxl wrote:<br>
> > > tejohnson wrote:<br>
> > > > davidxl wrote:<br>
> > > > > Can the name between bitcode file and final object files be made more related ?<br>
> > > > The ThinLink gold invocation doesn't know the name of the final object files. But note that if a prefix replace path is specified (thinlto-prefix-replace=oldprefix:newprefix plugin option), then the names emitted will have the matching prefix path replaced with the new one. This can be used to get the paths to the final object files into the file, as long as the final objects use the same names but a different path (true in bazel). Otherwise the build system would need to do post-processing of the list to map to the final object files it is going to create - known by the build system but not by gold here.<br>
> > > This request is more about test case readability. Suppose --start-lib and --end-lib pair includes two bit code files, but one of them is dropped in the first link. In the second step, we need to be able to tell which one maps to the kept one..<br>
> > Is the concern that it isn't obvious how %t.o and %t2.o map to lines in the CHECK? Note though that both are included in the link due to the gold version issue mentioned in the comments, so it should be pretty clear in this test case I think?<br>
</span><span>> yes for this case it is clear more or less.  A more general concern is that if we have a very large link that has a bug in the command line, how do we debug the problem? Whatever debugging facility we can use to trace the native .o back to the bitcode file can be used in tests like this.<br>
</span>I'm not following - that mechanism has to be in the build system. The separate processes in the distributed backend case mean that the two invocations of gold don't have a way to identify what the intervening clang invocations use as the output native object file name (the backend clang processes take the bitcode file and the index produced by the thin link as input, and an output file name, and invokes the importing/optimization pipelines).<br></blockquote><div><br></div></span><div>How does the clang invocation determine the name of the native output?  There is no fundamental reason we can not establish the name connection. If it is limited by the current implementation, we can revisit the issue later.</div></div></div></div></blockquote><div><br></div></div></div><div>Clang determines it based on the "-o filename" option passed to it. So it is entirely up to the build system. I don't think we want to hardcode an expectation in the compiler/plugin in the distributed backend case. It will limit flexibility on the build system side.</div></div></div></div></blockquote><div><br></div><div>Implementation wise, it is possible for the plugin to control and dump the name mapping in some file and let build system to honor it.  However as I said we can follow up on that later.</div><div><br></div><div>David</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>Teresa</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>David</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D22677" rel="noreferrer" target="_blank">https://reviews.llvm.org/D22677</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div><span class="HOEnZb"><font color="#888888">
</font></span></blockquote></div><span class="HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div class="m_8897444622567333599gmail_signature" data-smartmail="gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> <a href="tel:408-460-2413" value="+14084602413" target="_blank">408-460-2413</a></td></tr></tbody></table></span></div>
</font></span></div></div>
</blockquote></div><br></div></div>