<div dir="ltr"><div>> I do think we've got the bandwidth to implement the Manager classes at
this time, but that does seem like a good idea in the longer term to me.<br><br></div>Whoops, that should have been "don't think we've got the bandwidth".<br></div><div class="gmail_extra"><br><div class="gmail_quote">On 3 May 2017 at 10:17, James Henderson <span dir="ltr"><<a href="mailto:jh7370.2008@my.bristol.ac.uk" target="_blank">jh7370.2008@my.bristol.ac.uk</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><div><div>Ok, I'm happy to make a couple of changes based on the above comments. Here's my proposal:<br><br></div>1) Modify the timing information classes to provide a method to print and reset the counters other than at destruction time.<br></div>2) Add to the end of LTOCodeGenerator::<wbr>compileOptimized and ThinLTOCodeGenerator::run a call to the new method from 1).<br><br></div>I do think we've got the bandwidth to implement the Manager classes at this time, but that does seem like a good idea in the longer term to me.<span class="HOEnZb"><font color="#888888"><br><br></font></span></div><span class="HOEnZb"><font color="#888888">James<br></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On 3 May 2017 at 06:00, Mehdi AMINI <span dir="ltr"><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@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>2017-05-02 8:42 GMT-07:00 Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word">+Teresa, Mehdi<div><br><div><span class="m_2988607171457015435m_5761802945451564213gmail-"><blockquote type="cite"><div>On May 2, 2017, at 08:31, James Henderson <<a href="mailto:jh7370.2008@my.bristol.ac.uk" target="_blank">jh7370.2008@my.bristol.ac.uk</a>> wrote:</div><br class="m_2988607171457015435m_5761802945451564213gmail-m_1296828824151679719Apple-interchange-newline"><div><div dir="ltr"><p class="m_2988607171457015435m_5761802945451564213gmail-m_1296828824151679719gmail-MsoPlainText">Hi,</p><p class="m_2988607171457015435m_5761802945451564213gmail-m_1296828824151679719gmail-MsoPlainText">We have been investigating an issue when running LTO with
our proprietary linker, which links against libLTO dynamically. The issue is
that when we pass -time-passes via the lto_codegen_debug_options function in the
LTO C API, no time information is produced during compilation. The reason for
this is that time information is stored in state owned by a ManagedStatic
instance, and is only printed when the state is destroyed. This in turn only
happens when ManagedStatics are cleaned up, via the llvm_shutdown function. As
we do not link against LLVM (except libLTO dynamically), we have no access to
llvm_shutdown, which in turn means we are not able to clean up ManagedStatic
instances and thus no timing information is produced.</p><p class="m_2988607171457015435m_5761802945451564213gmail-m_1296828824151679719gmail-MsoPlainText">We have considered a few options and have come up with
the following suggestions, and would appreciate some feedback:</p><p class="m_2988607171457015435m_5761802945451564213gmail-m_1296828824151679719gmail-MsoPlainText" style="margin-left:36pt"><span><span>1)<span style="font-style:normal;font-variant-caps:normal;font-weight:normal;font-size:7pt;line-height:normal;font-family:'times new roman'"> </span></span></span>Add
llvm_shutdown (or rather likely some wrapper function that does the same job)
to the C interface of libLTO. This should be called when we are done with the
library.</p></div></div></blockquote></span><div>This seems pretty reasonable to me. I'm not sure what others think.</div><span class="m_2988607171457015435m_5761802945451564213gmail-"><blockquote type="cite"><div><div dir="ltr"><p class="m_2988607171457015435m_5761802945451564213gmail-m_1296828824151679719gmail-MsoPlainText" style="margin-left:36pt"><span><span>2)<span style="font-style:normal;font-variant-caps:normal;font-weight:normal;font-size:7pt;line-height:normal;font-family:'times new roman'"> </span></span></span>Add
a “full-shutdown” command-line option to LLVM - that can be passed via
lto_codegen_debug_options - which causes ManagedStatic-owned state to be
destroyed on shutdown. This could even be more widely useful outside the LTO
case.</p><p class="m_2988607171457015435m_5761802945451564213gmail-m_1296828824151679719gmail-MsoPlainText" style="margin-left:36pt"><span><span>3)<span style="font-style:normal;font-variant-caps:normal;font-weight:normal;font-size:7pt;line-height:normal;font-family:'times new roman'"> </span></span></span>Call
llvm_shutdown() immediately after compilation as part of the compile function.</p></div></div></blockquote></span></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><div><div><span class="m_2988607171457015435m_5761802945451564213gmail-"><blockquote type="cite"><div><div dir="ltr"><p class="m_2988607171457015435m_5761802945451564213gmail-m_1296828824151679719gmail-MsoPlainText" style="margin-left:36pt"><span><span>4)<span style="font-style:normal;font-variant-caps:normal;font-weight:normal;font-size:7pt;line-height:normal;font-family:'times new roman'"> </span></span></span>None
of the above, because there’s a better way that I am unaware of to clean up
this state.</p></div></div></blockquote></span></div></div></div></blockquote><div><br></div></span><div>In the short term, adding a way to print and reset the counters and call it at the end of the LTO process seems like a good immediate tradeoff to me. We're kind of doing it with statistics already: <a href="https://llvm.org/svn/llvm-project/llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp" target="_blank">https://llvm.org/svn/<wbr>llvm-project/llvm/trunk/lib/LT<wbr>O/ThinLTOCodeGenerator.cpp</a> (look at the very end of the file)</div><span><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word"><div><div><div>Perhaps this timing stuff should live in the LLVMContext instead? And then you get timing-per-LLVMContext?</div></div></div></div></blockquote><div><br></div><div><br></div></span><div>This last suggestion is my favorite answer by far!</div><div>I've been talking with Matthias about doing this for statistic as well: having a dedicated StatisticManager and TimerGroupManager (straw man names) that could live in the LLVMContext and have to be set explicitly by the client of the LLVMContext is what I'd aim for. </div><div>This is needed to solve the issue we have with ThinLTO and threading in general.</div><div><br></div><div>It is possible (likely...) that some places are using statistics without having a LLVMContext, but we should be able to thread through a StatisticManager somehow.</div><span class="m_2988607171457015435HOEnZb"><font color="#888888"><div><br></div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div><div><br></div></font></span></div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>