<div dir="ltr">The ELF writer in this patch seems a bit too abstracted to use only in LLD but a bit too less abstracted to be a generic library, so I'd think it'll have to be improved (to whichever). But this is not something we should worry too much about it, as you said, as this is a relatively small stuff.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 9, 2016 at 11:11 AM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.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">On Fri, Sep 9, 2016 at 10:47 AM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@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">Is that your plan?</div></blockquote><div><br></div><div>I'm not sure there is a "plan" per se, but Michael's patch has it as a local utility for the linker since that is how we have been using it. You can think about it similar to the CPIO utilities -- they start out as a local utility for ELF, then can be generalized to any other tools that need it. I haven't looked, but my guess is that llvm-dwp could use ELFCreator (or an evolved version) and potentially MC; also this may be useful for future tools like "llvm-strip" or "llvm-objcopy".</div><div><br></div><div>I don't think we should worry too much about where it will live. We can easily move it as needed.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_6899524901536868617HOEnZb"><div class="m_6899524901536868617h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 9, 2016 at 3:04 AM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.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 Thu, Sep 8, 2016 at 6:16 PM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@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">I'm a little confused. I was thinking that you wanted to upstream your local change to create ELF files in-memory to libSupport or something and then remove this code from LLD. Was I wrong?</div></blockquote><div><br></div></span><div>I don't have an issue with it living in libSupport (or potentially libObject since it fits there more naturally).</div><div><div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 8, 2016 at 6:00 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.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 Thu, Sep 8, 2016 at 5:17 PM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@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">OK, I don't want to make future upstreaming unnecessarily hard by requesting changes to that file, so I'm okay with that file structure. That said, please keep it in mind that this file would diverge from what it is today once we submit this to the open source repository because people would try to improve it, and we can't stop them if it is a good change. Even I would try to make changes to that file if upstreaming won't be done in a timely manner.</div></blockquote><div><br></div></span><div>Feel free to commit any genuine improvements to the code; we merge daily and will pick them up. Just please do so in the context that this is shared independent functionality, even though upstream may not reflect that.</div><span><font color="#888888"><div><br></div><div>-- Sean Silva</div></font></span><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 8, 2016 at 5:04 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.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"><div><div><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 8, 2016 at 3:19 PM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@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"><div class="gmail_extra"><div class="gmail_quote"><span>On Thu, Sep 8, 2016 at 3:14 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">silvas added a subscriber: silvas.<br>
<span><br>
================<br>
Comment at: ELF/InputFiles.cpp:736<br>
@@ +735,3 @@<br>
+template <class ELFT> std::unique_ptr<InputFile> BinaryFile::createELF() {<br>
+  ELFCreator<ELFT> ELF(ET_REL, Config->EMachine);<br>
+  auto DataSec = ELF.addSection(".data");<br>
----------------<br>
</span><span>ruiu wrote:<br>
> Bigcheese wrote:<br>
> > ruiu wrote:<br>
> > > How about this?<br>
> > Well, the code doesn't belong in ELFCreator.cpp. I can make a BinaryFile.cpp for it, but it's only 40 lines of code.<br>
> That distinction doesn't make much sense to me because this is part of the linker. If you have a concrete plan to move ELFCreator to some library, it may make sense, but it doesn't seem to happen soon.<br>
</span>Like Michael already said, ELFCreator is already used in at least 2 other places for PS4. It doesn't make sense to move this BinaryFile-specific code into ELFCreator.<br></blockquote><div><br></div></span><div>Is that change for PS4 upstreamed?</div></div></div></div>
</blockquote></div><br></div></div></div><div class="gmail_extra">Not yet. Right now, we are required to keep those private, but we try to do things so that they are ready to be sent upstream if we get the approval to do so (e.g. we merge the latest LLD daily).</div><span><font color="#888888"><div class="gmail_extra"><br></div><div class="gmail_extra">-- Sean Silva</div></font></span></div>
</blockquote></div><br></div>
</div></div></blockquote></span></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>