<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jul 19, 2017 at 8:52 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@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><br><div class="gmail_quote"><span class=""><div dir="ltr">On Wed, Jul 19, 2017 at 8:43 AM Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.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">On Wed, Jul 19, 2017 at 8:31 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@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><br><div class="gmail_quote"><span><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_3514511452371183091m_721936793358731472m_2773207081377780166m_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><span><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 - 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. Usually adding .ll formats doesn't seem to be terribly expensive/time intensive.<br></div></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>The main complication I see is defining the behavior when the serialized summary is read back in.</div><div>1) Do we trust that it is correct and consistent with the IR and blindly use it? That could cause some issues if someone changes the IR in a .ll file for testing and doesn't realize they need to also update the summary correspondingly.</div></div></div></div></blockquote></span><div><br>Generally textual IR is assumed to be bogus and is validated for invariants like this.<br></div></div></div></blockquote><div><br></div><div>So why have support to read it in from the .ll at all?</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>2) Do we always want to build the summary from the IR and check it against the summary read from the .ll file? In that case, what is even the use of building a summary from the serialized form?</div></div></div></div></blockquote></span><div><br>Isn't that why the YAML support was added in the first place - for reading in summaries for test cases?<br></div></div></div></blockquote><div><br></div><div>Peter added it for testing early ThinLTO CFI support since it predated the summary building support for these features.</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>3) If we want to allow tweaks to the summary in the .ll that override what is in the IR, for testing purposes, how does any checking we do in </div></div></div></div></blockquote></span><div><br>I'm not sure why we would - what would be tested that way? If the invariants of the summary are violated presumably the behavior of LLVM is undefined & so there's nothing to test/no expected/defined/usable behavior there.<br></div></div></div></blockquote><div><br></div><div>So again, why bother having support to serialize it back in at all? </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>2) distinguish between the case in 1) (user error) and 3) (intended difference)?</div><div><br></div><div>This is why I suggested emitting as comments in the .ll file initially (which is useful for debugging purposes, although the YAML works fine for that too), while the above are hashed out. </div></div></div></div></blockquote><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><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><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 (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></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Speaking for myself, it wasn't clear to me that serialization support was needed for anything other than debugging/testing,</div></div></div></div></blockquote></span><div><br>The textual IR in its entirety basically exists only for debugging/testing - but it's a big 'only'. (as has become apparent in this case with experience, by the looks of it)<br></div></div></div></blockquote><div><br></div><div>I think there is a big difference - for most of the IR, the information is not redundant - you simply can't create the same compiler behavior when reading a .ll. For the summary it is completely redundant as it is constructed from the IR.</div><div><br></div><div>Teresa</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><br>(not a criticism of you/your work - just that this should've been caught in code review, I think)<br> </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> since it is redundant with and computed from the IR, and I wasn't sure emitting into the .ll file was the right way for debugging. Which is why the testing used llvm-bcanalyzer -dump.</div><div><br></div><div>Teresa</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"><br>- Dave</div></div>
</blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><br><br clear="all"><div><br></div>-- <br><div class="m_3514511452371183091m_721936793358731472gmail_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)%20460-2413" value="+14084602413" target="_blank">408-460-2413</a></td></tr></tbody></table></span></div>
</div></div></blockquote></span></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_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"> 408-460-2413</td></tr></tbody></table></span></div>
</div></div>