<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 19, 2017 at 8:58 AM Teresa Johnson <<a href="mailto:tejohnson@google.com">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: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><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_6941878854320874209m_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></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>So why have support to read it in from the .ll at all?</div></div></div></div></blockquote><div><br>Fair question - if there's only a single right answer that's a bit different from a lot of (but not all) other .ll file constructs.<br><br>But, for example - the .ll syntax specifies the types of expressions at their use and verifies this. There's no ambiguity there, only one right answer, but it's still written in the IR and verified, makes the test case easier to read, perhaps (this may be a bad example - I certainly find that quirk of the IR a bit weird).<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"><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><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></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Peter added it for testing early ThinLTO CFI support since it predated the summary building support for these features.</div></div></div></div></blockquote><div><br>Ah, that's interesting to understand - thanks for explaining!<br><br>So does that make the current YAML reading support unneeded? (hopefully dead) or are old tests still written in it that need to be ported to some newer technique?<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"><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><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></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>So again, why bother having support to serialize it back in at all? </div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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><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></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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.</div></div></div></div></blockquote><div><br>I think there are a fair few constructs in the .ll file format that are redundant and exist for ease of use/readability/self-documenting tests & this seems like it'd fall under a similar category.<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"><div> For the summary it is completely redundant as it is constructed from the IR.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>Teresa</div></div></div></div><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><br>(not a criticism of you/your work - just that this should've been caught in code review, I think)<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"><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_6941878854320874209m_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></div></div><div dir="ltr"><div class="gmail_extra"><br><br clear="all"><div><br></div>-- <br><div class="m_6941878854320874209gmail_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></div></div>