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

[via llvm-commits]

Would it really be more efficient? From my experience iterating over bit arrays, I would much rather unconditionally bsr/bsf/tzcnt/lzcnt to find the next than have complex logic to handle consecutive bits. If you add branchy logic, you’ve lost the entire point of this kind of fast iteration (no branch mispredictions).

Fiona

> On Apr 13, 2017, at 5:58 PM, Matthias Braun via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> 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 <mailto: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>
>>> 
>>> 
>>> 
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

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