<p dir="ltr">Yes, the inlining info refers to function names in the type info. The test ensures that they correlate.</p>
<p dir="ltr">Readobj and pdbdump do use the same dumper library. I don't think it's particularly important to merge our testing tool dumpers, but I wouldn't be opposed to it.</p>
<p dir="ltr">Sent from phone</p>
<div class="gmail_quote">On Jun 3, 2016 6:03 PM, "David Blaikie" <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br type="attribution"><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">On Fri, Jun 3, 2016 at 8:43 AM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.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"><div class="gmail_extra"><div class="gmail_quote"><span>On Wed, Jun 1, 2016 at 10:22 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br></span><span><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_extra"><div class="gmail_quote"><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">+<br>
+  // Check if we've already translated this type.<br>
+  auto I = TypeIndices.find(Ty);<br>
+  if (I != TypeIndices.end())<br>
+    return I->second;<br>
+<br></blockquote><div><br></div></div></div><div>Might be worth adding a comment somewhere here to mention that lowerType might add new TypeIndices and thus you can't avoid the double lookup (find and then unhinted insert) here. Pretty obvious I guess, but seems nice to have.</div></div></div></div></blockquote><div><br></div></span><div>I added this comment in a later patch, and Amjad found it confusing:</div><div><a href="http://reviews.llvm.org/D20924#inline-177103" target="_blank">http://reviews.llvm.org/D20924#inline-177103</a></div></div></div></div></blockquote><div><br></div><div>Right - as you said there, I think that's just a matter of finding the right wording/place to put 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_extra"><div class="gmail_quote"><div><br></div><span><div><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 class="gmail_extra"><div class="gmail_quote"><div>At least for the DWARF tests we dwarfdump the output of LLVM and check against the pretty printed dump output. Is there a reason we need/prefer to test the asm directly here?</div></div></div></div></blockquote><div><br></div></span><div>inlining.ll already tests both the assembly output and the readobj output. The readobj output is a lot easier to read and validate for correctness, but I think it's also worth having a few tests of the assembly output, especially since inlining uses a bunch of custom directives.</div></div></div></div></blockquote><div><br></div><div>But this was a change for type info, right? Why would that have an impact on inlining? Does the inlined debug info with its custom directives reference the type hierarchy? (this isn't true in DWARF, where the inline info isn't part of the line table and the line table is very 'raw' - but I could imagine formats where it is true)<br><br>Does readobj do pretty dumping the same way pdbdump does? Seems a bit asymmetric with dwarfdump - should we roll dwarf dumping into readobj & kill dwarfdump for consistency? Or pull out CV record dumping? Or just is insufficiently analogous?</div><div> </div></div><br></div></div>
</blockquote></div>