[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