[PATCH] D32060: BitVector: add iterators for bits that are set

Matthias Braun via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 13 17:58:06 PDT 2017


Doing a `grep "find_first" * -r` over llvm shows you it is a common pattern, which IMO should be something we can do in 1 for. If anything you could rewrite the internal representation to work on strides, but I am not convinced it is worth the complexity today until someone shows me a case where it actually matters.

- Matthias

> On Apr 13, 2017, at 5:53 PM, Zachary Turner <zturner at google.com> wrote:
> 
> Would it really be that unhandy? You would only need to iterate from iter->first to iter->second to get the same effect.
> 
> Granted it wouldn't work with existing algorithms that take predicates or functions with integer arguments, but I wonder if anyone would even notice that limitation. And when the bits happen to be clustered together , which is not uncommon, it will be vastly more efficient 
> On Thu, Apr 13, 2017 at 5:44 PM Matthias Braun <matze at braunis.de <mailto:matze at braunis.de>> wrote:
> While more efficient, this seems unhandy to me...
> 
> Using a BitVector as a replacement for a std::set<unsigned> is a common pattern and in this case I would expect the iterator to return all the elements that I put into the set before...
> 
> - Matthias
> 
>> On Apr 13, 2017, at 5:41 PM, Zachary Turner <zturner at google.com <mailto:zturner at google.com>> wrote:
>> 
>> Instead of having the value type be an index, how about having it be a range? Eg if bits,1,2,3,6,8,9 are set then have it return (1,4), (6,7), and (8,10) on successive iterations. This should be much more efficient. For a bits_unset iterator, the above would return (0,1), (4, 6), (7,8), (10, size())
>> On Thu, Apr 13, 2017 at 5:29 PM Matthias Braun via Phabricator <reviews at reviews.llvm.org <mailto:reviews at reviews.llvm.org>> wrote:
>> MatzeB added reviewers: dblaikie, chandlerc, mehdi_amini.
>> MatzeB added a comment.
>> 
>> Adding a few reviewers who often have comments on ADT changes.
>> 
>> 
>> https://reviews.llvm.org/D32060 <https://reviews.llvm.org/D32060>
>> 
>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170413/bb962038/attachment.html>


More information about the llvm-commits mailing list