[llvm] [ADT] Make set_subtract more efficient when subtrahend is larger (NFC) (PR #98702)
Kazu Hirata via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 12 23:55:24 PDT 2024
================
@@ -92,9 +92,27 @@ S1Ty set_difference(const S1Ty &S1, const S2Ty &S2) {
return Result;
}
+/// set_subtract_vec(A, B) - Compute A := A - B, where B can be a vector.
+///
+template <class S1Ty, class S2Ty>
+void set_subtract_vec(S1Ty &S1, const S2Ty &S2) {
+ for (typename S2Ty::const_iterator SI = S2.begin(), SE = S2.end(); SI != SE;
+ ++SI)
+ S1.erase(*SI);
+}
+
/// set_subtract(A, B) - Compute A := A - B
///
+/// Selects the set to iterate based on the relative sizes of A and B for better
+/// efficiency.
+///
template <class S1Ty, class S2Ty> void set_subtract(S1Ty &S1, const S2Ty &S2) {
+ if (S1.size() < S2.size()) {
+ for (typename S1Ty::iterator SI = S1.begin(), SE = S1.end(); SI != SE; ++SI)
+ if (S2.count(*SI))
+ S1.erase(SI);
+ return;
+ }
----------------
kazutakahirata wrote:
I wonder if we could teach `set_subtract` to use the new path `S1.size() < S2.size()` when either `S2Ty::contains` or `S2Ty::find` is available:
```suggestion
using ElemTy = decltype(*S1.begin());
if constexpr (detail::HasMemberContains<S2Ty, ElemTy> ||
detail::HasMemberFind<S2Ty, ElemTy>) {
if (S1.size() < S2.size()) {
for (typename S1Ty::iterator SI = S1.begin(), SE = S1.end(); SI != SE; ++SI)
if (llvm::is_contained(S2, *SI))
S1.erase(SI);
return;
}
}
```
This way, we don't have to have `set_subtract_vec`. If `S2Ty` is a vector, the `constexpr if` would automatically drop the new path. I'm using `detail::HasMemberContains<S2Ty, ElemTy>` and `detail::HasMemberFind<S2Ty, ElemTy>` as a heuristic to see if `S2Ty` offers a sublinear lookup like O(1) or O(log(n)). Calling `is_contained` without the `constexpr if` would be a disaster, invoking a linear look up when `S2Ty` is a vector.
@kuhar Any thoughts here? I recall you generalized `is_contained` a while ago at 0eaacc25bb98f50cc98bab5f6ef8d6d67e112317.
https://github.com/llvm/llvm-project/pull/98702
More information about the llvm-commits
mailing list