[PATCH] D156005: Use `getHashValue` in `SetVector::insert` and `SetVector::contains`

Dhruv Chawla via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 21 19:56:33 PDT 2023


0xdc03 added a comment.

Adding onto what `@efriedma` said, if you do want to use hash-value equality I would suggest having value equality as well, chained using `&&`. Given that the "small" functionality is meant to prevent us from having to compute a hash value in the first place, I would also suggest you to measure this change on the compile time tracker, although I don't expect it to make the performance better especially because this is quite a performance-sensitive part of code...



================
Comment at: llvm/include/llvm/ADT/SetVector.h:276
       if (isSmall())
-        return is_contained(vector_, key);
+        return contains(key);
 
----------------
etcwilde wrote:
> We can probably reduce this entire function down to a call to `contains` since that returns a boolean and this only returns `0` or `1`. Thoughts?
That makes sense to me, the reason I didn't do that was to keep the diff as insertions-only and the name `count` also confused me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156005/new/

https://reviews.llvm.org/D156005



More information about the llvm-commits mailing list