[PATCH] [PR20893] Change SelectionDAG's DbgValueMap to have set semantics (NFC)

[David Blaikie]

On Mon, Sep 22, 2014 at 3:14 PM, Adrian Prantl <aprantl at apple.com> wrote:

>
> > On Sep 22, 2014, at 2:18 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > as for testing - assuming these duplicates get deduplicated elsewhere
> (could you explain where?)
> There are two kinds of duplicates that appear in this case:
> 1) identical SDDbgNode* (same pointer value)
> 2) an equivalent SDDbgNode (distinct pointers, but otherwise a copy)
> The first kind is automatically deduplicated because the Invalid flag is
> set after processing it the first time.
> The second one will result in two duplicate DBG_VALUE intrinsics which
> will get coalesced by the backend.
>

OK - perhaps you could explain how these two equivalent but distinct
SDDbgNodes come into existence?


>
> > then I doubt there's much to test - this is then just a perf
> optimization, not a change in any semantics.
> I was mostly posting this here seeking absolution for not committing this
> with a testcase :-)
> > 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.
> 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.
>

You can't cause two of these equivalent but distinct objects to be created
with less than several thousand lines?

That is a bit surprising/strange.


>
> thanks,
> adrian
> >
> > ================
> > Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:6180
> > @@ +6179,3 @@
> > +      if (*Val == *V) return;
> > +    Vals.push_back(V);
> > +  }
> > ----------------
> > Could 'Vals' be an actual set of SDDbgValue*s?
> >
> > 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)
> >
> > http://reviews.llvm.org/D5451
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140922/876e488a/attachment.html>