<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">Hi Benoit,</div><div class="gmail_quote"><br></div><div class="gmail_quote">On Tue, Jun 20, 2017 at 3:45 PM, Benoit Belley <span dir="ltr"><<a href="mailto:Benoit.Belley@autodesk.com" target="_blank">Benoit.Belley@autodesk.com</a>></span> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Thanks Peter, this is very useful feedback.<br>
<br>
I did manage to change the behavior of LinkOnlyNeeded to correctly import<br>
all variables with AppendingLinkage. In fact, I discovered that there was<br>
already something fishy. A variable with AppendingLinkage would get<br>
imported correctly from the source module if the destination module<br>
already contained a definition for that variable and wouldn't be imported<br>
otherwiseŠ My local fix ensures that it correctly gets imported in both<br>
cases.<br></blockquote><div><br></div><div>Thanks, this does sound even more like the right fix to me then.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
You are right; ThinLTO no longer uses the Linker class. I was able to<br>
remove the useless include of Linker.h from ThinLTOCodeGenerator.cpp.<br>
<br>
That being said, Linker.h and LinkModules.cpp  still have a few comments<br>
about ThinLTO, namely:<br>
<br>
  /// \brief Link \p Src into the composite.<br>
  ///<br>
  /// Passing OverrideSymbols as true will have symbols from Src<br>
  /// shadow those in the Dest.<br>
  /// For ThinLTO function importing/exporting the \p ModuleSummaryIndex<br>
  /// is passed. If \p GlobalsToImport is provided, only the globals that<br>
  /// are part of the set will be imported from the source module.<br>
  ///<br>
  /// Returns true on error.<br>
<br>
<br>
  // Don't want to append to global_ctors list, for example, when we<br>
  // are importing for ThinLTO, otherwise the global ctors and dtors<br>
  // get executed multiple times for local variables (the latter causing<br>
  // double frees).<br>
<br>
<br>
    // For ThinLTO we don't import more than what was required.<br>
    // The client has to guarantee that the linkonce will be availabe at<br>
link<br>
    // time (by promoting it to weak for instance).<br>
<br>
<br>
Are these obsolete comments ? Should these be cleaned-up ? If so, how ?<br></blockquote><div><br></div><div>Are you sure that you are looking at the latest version of the code? I believe that I removed those comments (and the associated code) in r294015.</div><div><br></div><div>Peter</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Thanks again,<br>
Benoit<br>
<br>
<br>
From:  Peter Collingbourne <<a href="mailto:peter@pcc.me.uk" target="_blank">peter@pcc.me.uk</a>><br>
Date:  mardi 20 juin 2017 à 13:51<br>
To:  Benoit Belley <<a href="mailto:benoit.belley@autodesk.com" target="_blank">benoit.belley@autodesk.com</a>><br>
Cc:  David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>>, llvm-dev<br>
<span><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>, Lang Hames <<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>>, Teresa Johnson<br>
<<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>><br>
Subject:  Re: [llvm-dev] JIT, LTO and @llvm.global_ctors: Looking for<br>
advise<br>
<br>
<br>
</span><span>>Hi Benoit,<br>
><br>
>It seems to me that LinkOnlyNeeded failing to link the llvm.* special<br>
>variables is a bug. I think we should probably just change the behaviour<br>
>of LinkOnlyNeeded so that it links them.<br>
><br>
>Note that ThinLTO does not use the Linker class, it uses IRMover<br>
>directly. That might not be appropriate for your use case, though,<br>
>because IRMover does not follow references when linking (for example, if<br>
>your module defines two functions f and g, where<br>
> f calls g, it will not link g if you only ask for f).<br>
><br>
>Peter<br>
><br>
><br>
>On Tue, Jun 20, 2017 at 8:38 AM, Benoit Belley via llvm-dev<br>
><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
><br>
>Thanks for the hindsight.<br>
><br>
>I am currently working on a patch/potential fix which introduces a new<br>
>Linker::ImportIntrinsicGlobal<wbr>Variables flag. The patch includes a unit<br>
>test reproducing the problem. Hopefully, that will help getting more<br>
>feedback.<br>
><br>
>Note that it might take a while before I am allowed to upload the patch<br>
>since I need approval from Autodesk Legal department.<br>
><br>
>Cheers,<br>
>Benoit<br>
><br>
>Benoit Belley<br>
>Sr Principal Developer<br>
>M&E-Product Development Group<br>
><br>
</span>>MAIN <a href="tel:%2B1%20514%20393%201616" value="+15143931616" target="_blank">+1 514 393 1616</a> <tel:%2B1%20514%20393%201616><br>
>DIRECT <a href="tel:%2B1%20438%20448%206304" value="+14384486304" target="_blank">+1 438 448 6304</a> <tel:%2B1%20438%20448%206304><br>
>FAX <a href="tel:(514)%20393-0110" value="+15143930110" target="_blank">+1 514 393 0110</a> <tel:%2B1%20514%20393%200110><br>
<span>><br>
>Twitter <<a href="http://twitter.com/autodesk" rel="noreferrer" target="_blank">http://twitter.com/autodesk</a>><br>
>Facebook <<a href="https://www.facebook.com/Autodesk" rel="noreferrer" target="_blank">https://www.facebook.com/Auto<wbr>desk</a>><br>
><br>
>Autodesk, Inc.<br>
>10 Duke Street<br>
>Montreal, Quebec, Canada H3C 2L7<br>
><a href="http://www.autodesk.com" rel="noreferrer" target="_blank">www.autodesk.com</a> <<a href="http://www.autodesk.com" rel="noreferrer" target="_blank">http://www.autodesk.com</a>> <<a href="http://www.autodesk.com/" rel="noreferrer" target="_blank">http://www.autodesk.com/</a>><br>
><br>
><br>
><br>
><br>
><br>
</span><div><div class="gmail-m_-7846910157427730425m_-308904951313077194h5">>From:  David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>><br>
>Date:  mardi 20 juin 2017 à 11:12<br>
>To:  Benoit Belley <<a href="mailto:benoit.belley@autodesk.com" target="_blank">benoit.belley@autodesk.com</a>>, llvm-dev<br>
><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>, Lang Hames <<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>>, Teresa Johnson<br>
><<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>><br>
>Subject:  Re: [llvm-dev] JIT, LTO and @llvm.global_ctors: Looking for<br>
>advise<br>
><br>
><br>
>>+Lang (for JIT) & Teresa (for LTO/ThinLTO).<br>
>><br>
>>Sounds like maybe the LinkOnlyNeeded got reused for a bit more than the<br>
>>original intent & maybe there should be more than one flag here - not<br>
>>sure.<br>
>><br>
>>On Mon, Jun 19, 2017 at 9:16 AM Benoit Belley via llvm-dev<br>
>><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
>><br>
>><br>
>>Hi Everyone,<br>
>><br>
>>We are looking for advise regarding the proper use of LTO in<br>
>>conjunction with just-in time generated code. Our usage scenario goes<br>
>>as follows.<br>
>><br>
>>  1. Our front-end generates an LLVM module.<br>
>><br>
>>  2. A small runtime support library is linked-in. The runtime<br>
>>     library is distributed as bitcode. It is generated using "clang++<br>
>>     -emit-llvm' and 'llvm-link'. This allows LTO to kick-in and<br>
>>     functions from the runtime support library become candidates for<br>
>>     inlining. This is the desired effect that we are looking for.<br>
>><br>
>>  3. The resulting LLVM module is compiled and dynamically loaded. We<br>
>>     are currently using the MCJIT API, but are planning to move to ORC<br>
>>     very soon.<br>
>><br>
>>Our LLVM module linking code goes roughly as follows:<br>
>><br>
>>  Linker linker(jittedModule);<br>
>>  std::unique_ptr<llvm::Module> moduleToLink(<br>
>>    getLazyIRFileModule(bcFileName<wbr>, error, context));<br>
>>  linker.linkInModule(std::move(<wbr>module),<br>
>>                      Linker::LinkOnlyNeeded |<br>
>>                      Linker::InternalizeLinkedSymbo<wbr>l);<br>
>><br>
>>Our issue is with the Linker::LinkOnlyNeeded flag. Using it has a huge<br>
>>positive impact on link and compilation time :-). But, it causes the<br>
>>@llvm.global_ctors and @llvm.global_dtors references from the<br>
>>linked-in modules to be discarded :-(. AFAICT, the Linker code assumes<br>
>>ThinLTO when the LinkOnlyNeeded flags is specified, and full-LTO<br>
>>otherwise.<br>
>><br>
>>To resolve this, we have locally patched<br>
>>llvm/lib/Linker/LinkModules.<wbr>cpp with:<br>
>><br>
>>  bool ModuleLinker::run() {<br>
>><br>
>>    // ....<br>
>><br>
>>    if (shouldImportIntrinsicGlobalVa<wbr>riables()) {<br>
>>      auto addIntrinsicGlobalVariable = [ValuesToLink,<br>
>>                                         srcM](llvm::StringRef name) {<br>
>>        if (GlobalValue *GV = SrcM->getGlobalVariable(name)) {<br>
>>          ValuesToLink.insert(GV);<br>
>>        }<br>
>>      };<br>
>><br>
>>      // These are added here, because they must not be internalized.<br>
>>      addIntrinsicGlobalVariable("ll<wbr>vm.used");<br>
>>      addIntrinsicGlobalVariable("ll<wbr>vm.compiler.used");<br>
>>      addIntrinsicGlobalVariable("ll<wbr>vm.global_ctors");<br>
>>      addIntrinsicGlobalVariable("ll<wbr>vm.global_dtors");<br>
>>    }<br>
>><br>
>>    // ...<br>
>><br>
>>  }<br>
>><br>
>>Questions:<br>
>><br>
>>   1. Is attempting to use llvm::Linker appropriate for our usage<br>
>>      pattern ? Should we directly use llvm::IRMover instead ?<br>
>><br>
>>   2. Or, is our patch to ModuleLinker::run() the way to go ? Should<br>
>>      we submit back a patch along these lines ?<br>
>><br>
>>   3. Other suggestions ?<br>
>><br>
>>[Note] We are currently using LLVM 4.0.1-rc2.<br>
>><br>
>>Cheers,<br>
>>Benoit<br>
>><br>
>><br>
>><br>
>>Benoit Belley<br>
>>Sr Principal Developer<br>
>>M&E-Product Development Group<br>
>><br>
><br>
><br>
</div></div>>>MAIN <a href="tel:%2B1%20514%20393%201616" value="+15143931616" target="_blank">+1 514 393 1616</a> <tel:%2B1%20514%20393%201616> <tel:(514)%20393-1616><br>
>>DIRECT <a href="tel:%2B1%20438%20448%206304" value="+14384486304" target="_blank">+1 438 448 6304</a> <tel:%2B1%20438%20448%206304><br>
>><tel:(438)%20448-6304><br>
>>FAX <a href="tel:(514)%20393-0110" value="+15143930110" target="_blank">+1 514 393 0110</a> <tel:%2B1%20514%20393%200110> <tel:(514)%20393-0110><br>
<span>>><br>
>>Twitter <<a href="http://twitter.com/autodesk" rel="noreferrer" target="_blank">http://twitter.com/autodesk</a>><br>
>>Facebook <<a href="https://www.facebook.com/Autodesk" rel="noreferrer" target="_blank">https://www.facebook.com/Auto<wbr>desk</a>><br>
>><br>
>>Autodesk, Inc.<br>
>>10 Duke Street<br>
>>Montreal, Quebec, Canada H3C 2L7<br>
</span>>><a href="http://www.autodesk.com" rel="noreferrer" target="_blank">www.autodesk.com</a> <<a href="http://www.autodesk.com" rel="noreferrer" target="_blank">http://www.autodesk.com</a>> <<a href="http://www.autodesk.com" rel="noreferrer" target="_blank">http://www.autodesk.com</a>><br>
<div class="gmail-m_-7846910157427730425m_-308904951313077194HOEnZb"><div class="gmail-m_-7846910157427730425m_-308904951313077194h5">>><<a href="http://www.autodesk.com/" rel="noreferrer" target="_blank">http://www.autodesk.com/</a>><br>
>><br>
>><br>
>>____________________________<wbr>___________________<br>
>>LLVM Developers mailing list<br>
>><a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
>><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bi<wbr>n/mailman/listinfo/llvm-dev</a><br>
><br>
>_____________________________<wbr>__________________<br>
>LLVM Developers mailing list<br>
><a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin<wbr>/mailman/listinfo/llvm-dev</a><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
>--<br>
>--<br>
>Peter<br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail-m_-7846910157427730425m_-308904951313077194gmail_signature"><div dir="ltr">-- <div>Peter</div></div></div>
</div></div>