<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 22, 2014 at 3:14 PM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> On Sep 22, 2014, at 2:18 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
> as for testing - assuming these duplicates get deduplicated elsewhere (could you explain where?)<br>
</span>There are two kinds of duplicates that appear in this case:<br>
1) identical SDDbgNode* (same pointer value)<br>
2) an equivalent SDDbgNode (distinct pointers, but otherwise a copy)<br>
The first kind is automatically deduplicated because the Invalid flag is set after processing it the first time.<br>
The second one will result in two duplicate DBG_VALUE intrinsics which will get coalesced by the backend.<br></blockquote><div><br></div><div>OK - perhaps you could explain how these two equivalent but distinct SDDbgNodes come into existence?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> then I doubt there's much to test - this is then just a perf optimization, not a change in any semantics.<br>
</span>I was mostly posting this here seeking absolution for not committing this with a testcase :-)<br>
<span class="">> Maybe dumping MI could be testable, but I'm not sure what the dumping options (& whether they're available in non-asserts builds, etc) are like.<br>
</span>There is -print-after-all and the likes, but the real problem is that the testcase is too large to serve as a useful regression test.<br></blockquote><div><br></div><div>You can't cause two of these equivalent but distinct objects to be created with less than several thousand lines? </div><div><br></div><div>That is a bit surprising/strange.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
thanks,<br>
adrian<br>
<div class="HOEnZb"><div class="h5">><br>
> ================<br>
> Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:6180<br>
> @@ +6179,3 @@<br>
> +      if (*Val == *V) return;<br>
> +    Vals.push_back(V);<br>
> +  }<br>
> ----------------<br>
> Could 'Vals' be an actual set of SDDbgValue*s?<br>
><br>
> Are SDDbgValue*s inherently uniqued such that pointer identity would be sufficient? (or do you need that equality comparison added above? Which could hopefully be adjusted to < comparison (or hashing) for a set-like structure)<br>
><br>
> <a href="http://reviews.llvm.org/D5451" target="_blank">http://reviews.llvm.org/D5451</a><br>
><br>
><br>
<br>
</div></div></blockquote></div><br></div></div>