[PATCH] D54007: Use a data structure better suited for large sets in SimplificationTracker.

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In https://reviews.llvm.org/D54007#1289855, @skatkov wrote:

> Just a note, there is a specific usage of this data structure. 
>  Phi nodes are inserted in the initial step without any removals and then only removals are used.
>
> So we should not see something like delete-insert.
>  So while data structure really suffers from what Bjorn wrote but in this specific application of this data structure I do not expect any problems...


Ok. Just worried that someone might find this set implementation nifty, trying to reuse it in some other part of the code or moving it to ADT, without considering this limitation regarding delete-insert.
If the extra cleanup in the NodeList is costly we could at least add some comment explaining this in the description of the set (and/or in the erase method). And maybe we can assert somehow that there are no inserts after erase (at insert the size of the NodeList and the size of the NodeSet must be equal)?
Still, if there is no big performance issue with making the data structure more general (also supporting delete-insert), such a solution could be accepatable as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D54007