<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2017-07-19 8:31 GMT-07:00 David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><span class=""><div dir="ltr">On Mon, Jul 17, 2017 at 5:18 PM Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>> wrote:<br></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">2017-07-17 16:49 GMT-07:00 David Blaikie via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</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 dir="ltr"><br><br><div class="gmail_quote"><span class="m_2004464920861374485m_353884039871643744gmail-"><div dir="ltr">On Mon, Jul 17, 2017 at 6:11 AM Charles Saternos via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></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 dir="ltr"><div><span style="font-size:12.8px">Hey @chandlerc and @dblaikie,</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Any updates on this in relation to "</span>[PATCH] D34080: [ThinLTO] Add dump-summary command to llvm-lto2 tool"?</div></div></blockquote></span><div><br>Sorry you've kind of got stuck in the middle of this - but I'm still hoping to hear/understand the pushback on implementing this as a first class .ll construct with serialization and deserialization support.<br><br>I think Peter mentioned he didn't think this was the right path forward in the long term? If that's the case, I'd like to understand that/reach that conclusion for the project now rather than treating this as a stop-gap with some idea that in the future someone might implement full serialization support (when it's been over a year already, and other stop gaps have been implemented (the yaml input support) already).<br></div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>I'm totally believing we need first class serialization support in .ll, and I have a path forward for this (just not a lot of time to dedicate to this).</div></div></div></div></blockquote></span><div><br>What's the rough expectation of time/complexity for this path forward?<br></div></div></div></blockquote><div><br></div><div>It depends, I think that either:</div><div><br></div><div>1) We unblock the progress on the independent YAML side, so that folks can work / debug efficiently. We have time to hammer the right syntax and implement it for .ll</div><div>2) We don't move forward with the independent YAML files, but since we need something to be able to work with, we start by integrating YAML in .ll so that we have the round-trip feature available ASAP, and folks can debug as well. We can then gradually move to another syntax since .ll format isn't stable.</div><div><br></div><div>I'm quite unhappy that this is been stalled right now.</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_quote"><span class=""><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"><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 dir="ltr"><div class="gmail_quote"><div>& if a .ll construct with serialization/deserialization is the path forward, understanding the motivation for a something other than going straight for that would be helpful -usually bitcode features come with .ll support from day 1, not a year later. I'm not clear on what would make this feature an exception/more expensive to do this for (& why it would be worth deferring that work, and what/when that work will be motivated/done)</div></div></div></blockquote><div><br></div><div><br></div><div><div>We need a debugging tool for summaries ASAP, and the YAML is *already* implemented.</div></div></div></div></div></blockquote></span><div><br>I'm not sure I understand why the tradeoff is worthwhile - </div></div></div></blockquote><div><br></div><div>Because (as Teresa mentioned separately) debugging issues with llvm-bc-analyzer is terrible.</div><div> <br></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_quote"><div>in terms of needing to add a new feature (even if it's already implemented) and tests, then porting those tests to a first-class .ll construct later. </div></div></div></blockquote><div><br></div><div>The feature we're talking about here is just the dump part: i.e. it provides a debugging tool for developer. It does not allow to write test that would read it.</div><div>This is helpful to anyone that has to understand "what's going on" when there something unexpected with ThinLTO.</div><div> <br></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_quote"><div>Usually adding .ll formats doesn't seem to be terribly expensive/time intensive.<br></div></div></div></blockquote><div><br></div><div>I'm fine if you want to do it.</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_quote"><div> </div><span class=""><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><div> Making it available through the llvm-lto tool is a no-brainer to me.</div><div><br></div><div>This was *not* an oversight but a deliberate choice to not do this in the first place. Because summaries are the first bitcode feature I know of that isn't attached in any way to a Module (you can't get to it from a Module).</div></div></div></div></div></blockquote><div><br></div></span>Not sure I quite follow why that difference made the choice/tradeoff here different</div></div></blockquote><div><br></div><div>You keep mentioning that the lack of .ll was an oversight:</div><div>1) why are you writing that if it does not make any difference for the current discussion?</div><div>2) I had to rectify the facts at some point.</div><div><br></div><div> <br></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_quote"> (which admittedly is a bit easier to see in retrospect maybe - now that there's been a need to build serialization and deserialization). Do you mean it wasn't clear that serialization support was needed when summaries were first implemented, but it is clear now?<br></div></div></blockquote><div><br></div><div>You mean *textual* serialization I assume (we have bitcode serialization from the beginning of course).</div><div>So, no it wasn't clear that it was *needed*, I think we saw it as "nice to have". We talked about it multiple times, but we figured that since the summaries a produced from the textual IR, you can write your test in IR and generate the summaries on demand. That got us a long way!</div><div>Debugging has been quite annoying though (llvm-bc-analyzer and custom "printf" here and there).</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div></div></div></div>