From 0ca51b6b66d081d8fcbcf9d715f9be8e22b0d609 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Mon, 23 Aug 2021 16:13:17 -0700 Subject: [PATCH 01/19] Make SparseBitMatrix::ensure_row public to enable general mutation of rows --- compiler/rustc_index/src/bit_set.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index df777502c44a4..282bed274e665 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -956,7 +956,7 @@ impl SparseBitMatrix { Self { num_columns, rows: IndexVec::new() } } - fn ensure_row(&mut self, row: R) -> &mut HybridBitSet { + pub fn ensure_row(&mut self, row: R) -> &mut HybridBitSet { // Instantiate any missing rows up to and including row `row` with an // empty HybridBitSet. self.rows.ensure_contains_elem(row, || None); From 79e0a0faf93922f77f7658d2b1b95c79c9bd9608 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 24 Aug 2021 17:50:08 -0700 Subject: [PATCH 02/19] Refactor BitSet relational methods into trait with parameterized right-hand side --- compiler/rustc_index/src/bit_set.rs | 353 ++++++++++++------ .../src/borrow_check/region_infer/values.rs | 4 +- compiler/rustc_mir/src/transform/generator.rs | 4 +- 3 files changed, 235 insertions(+), 126 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 282bed274e665..0b5745072ad11 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -16,6 +16,43 @@ pub type Word = u64; pub const WORD_BYTES: usize = mem::size_of::(); pub const WORD_BITS: usize = WORD_BYTES * 8; +pub trait BitRelations { + fn union(&mut self, other: &Rhs) -> bool; + fn subtract(&mut self, other: &Rhs) -> bool; + fn intersect(&mut self, other: &Rhs) -> bool; +} + +macro_rules! bit_relations_inherent_impls { + () => { + /// Sets `self = self | other` and returns `true` if `self` changed + /// (i.e., if new bits were added). + pub fn union(&mut self, other: &Rhs) -> bool + where + Self: BitRelations, + { + >::union(self, other) + } + + /// Sets `self = self - other` and returns `true` if `self` changed. + /// (i.e., if any bits were removed). + pub fn subtract(&mut self, other: &Rhs) -> bool + where + Self: BitRelations, + { + >::subtract(self, other) + } + + /// Sets `self = self & other` and return `true` if `self` changed. + /// (i.e., if any bits were removed). + pub fn intersect(&mut self, other: &Rhs) -> bool + where + Self: BitRelations, + { + >::intersect(self, other) + } + }; +} + /// A fixed-size bitset type with a dense representation. /// /// NOTE: Use [`GrowableBitSet`] if you need support for resizing after creation. @@ -134,25 +171,6 @@ impl BitSet { new_word != word } - /// Sets `self = self | other` and returns `true` if `self` changed - /// (i.e., if new bits were added). - pub fn union(&mut self, other: &impl UnionIntoBitSet) -> bool { - other.union_into(self) - } - - /// Sets `self = self - other` and returns `true` if `self` changed. - /// (i.e., if any bits were removed). - pub fn subtract(&mut self, other: &impl SubtractFromBitSet) -> bool { - other.subtract_from(self) - } - - /// Sets `self = self & other` and return `true` if `self` changed. - /// (i.e., if any bits were removed). - pub fn intersect(&mut self, other: &BitSet) -> bool { - assert_eq!(self.domain_size, other.domain_size); - bitwise(&mut self.words, &other.words, |a, b| a & b) - } - /// Gets a slice of the underlying words. pub fn words(&self) -> &[Word] { &self.words @@ -208,33 +226,167 @@ impl BitSet { not_already } + + bit_relations_inherent_impls! {} } -/// This is implemented by all the bitsets so that BitSet::union() can be -/// passed any type of bitset. -pub trait UnionIntoBitSet { - // Performs `other = other | self`. - fn union_into(&self, other: &mut BitSet) -> bool; +impl BitRelations> for BitSet { + fn union(&mut self, other: &BitSet) -> bool { + assert_eq!(self.domain_size, other.domain_size); + bitwise(&mut self.words, &other.words, |a, b| a | b) + } + + fn subtract(&mut self, other: &BitSet) -> bool { + assert_eq!(self.domain_size, other.domain_size); + bitwise(&mut self.words, &other.words, |a, b| a & !b) + } + + fn intersect(&mut self, other: &BitSet) -> bool { + assert_eq!(self.domain_size, other.domain_size); + bitwise(&mut self.words, &other.words, |a, b| a & b) + } } -/// This is implemented by all the bitsets so that BitSet::subtract() can be -/// passed any type of bitset. -pub trait SubtractFromBitSet { - // Performs `other = other - self`. - fn subtract_from(&self, other: &mut BitSet) -> bool; +fn sequential_update(mut f: impl FnMut(T) -> bool, it: impl Iterator) -> bool { + let mut changed = false; + for elem in it { + changed |= f(elem); + } + changed } -impl UnionIntoBitSet for BitSet { - fn union_into(&self, other: &mut BitSet) -> bool { - assert_eq!(self.domain_size, other.domain_size); - bitwise(&mut other.words, &self.words, |a, b| a | b) +fn sparse_intersect( + set: &mut SparseBitSet, + other_contains: impl Fn(&T) -> bool, +) -> bool { + let mut changed = false; + for i in (0..set.len()).rev() { + if !other_contains(&set.elems[i]) { + set.elems.remove(i); + changed = true; + } } + changed } -impl SubtractFromBitSet for BitSet { - fn subtract_from(&self, other: &mut BitSet) -> bool { - assert_eq!(self.domain_size, other.domain_size); - bitwise(&mut other.words, &self.words, |a, b| a & !b) +impl BitRelations> for BitSet { + fn union(&mut self, other: &SparseBitSet) -> bool { + sequential_update(|elem| self.insert(elem), other.iter().cloned()) + } + + fn subtract(&mut self, other: &SparseBitSet) -> bool { + sequential_update(|elem| self.remove(elem), other.iter().cloned()) + } + + fn intersect(&mut self, other: &SparseBitSet) -> bool { + self.intersect(&other.to_dense()) + } +} + +impl BitRelations> for SparseBitSet { + fn union(&mut self, other: &BitSet) -> bool { + sequential_update(|elem| self.insert(elem), other.iter()) + } + + fn subtract(&mut self, other: &BitSet) -> bool { + sequential_update(|elem| self.remove(elem), other.iter()) + } + + fn intersect(&mut self, other: &BitSet) -> bool { + sparse_intersect(self, |el| other.contains(*el)) + } +} + +impl BitRelations> for SparseBitSet { + fn union(&mut self, other: &SparseBitSet) -> bool { + sequential_update(|elem| self.insert(elem), other.iter().cloned()) + } + + fn subtract(&mut self, other: &SparseBitSet) -> bool { + sequential_update(|elem| self.insert(elem), other.iter().cloned()) + } + + fn intersect(&mut self, other: &SparseBitSet) -> bool { + sparse_intersect(self, |el| other.contains(*el)) + } +} + +impl BitRelations> for S +where + S: BitRelations> + BitRelations>, +{ + fn union(&mut self, other: &HybridBitSet) -> bool { + match other { + HybridBitSet::Sparse(sparse) => self.union(sparse), + HybridBitSet::Dense(dense) => self.union(dense), + } + } + + fn subtract(&mut self, other: &HybridBitSet) -> bool { + match other { + HybridBitSet::Sparse(sparse) => self.subtract(sparse), + HybridBitSet::Dense(dense) => self.subtract(dense), + } + } + + fn intersect(&mut self, other: &HybridBitSet) -> bool { + match other { + HybridBitSet::Sparse(sparse) => self.intersect(sparse), + HybridBitSet::Dense(dense) => self.intersect(dense), + } + } +} + +impl BitRelations> for HybridBitSet { + fn union(&mut self, other: &HybridBitSet) -> bool { + match self { + HybridBitSet::Sparse(self_sparse) => { + match other { + HybridBitSet::Sparse(other_sparse) => self_sparse.union(other_sparse), + + HybridBitSet::Dense(other_dense) => { + // `self` is sparse and `other` is dense. To + // merge them, we have two available strategies: + // * Densify `self` then merge other + // * Clone other then integrate bits from `self` + // The second strategy requires dedicated method + // since the usual `union` returns the wrong + // result. In the dedicated case the computation + // is slightly faster if the bits of the sparse + // bitset map to only few words of the dense + // representation, i.e. indices are near each + // other. + // + // Benchmarking seems to suggest that the second + // option is worth it. + let mut new_dense = other_dense.clone(); + let changed = new_dense.reverse_union_sparse(self_sparse); + *self = HybridBitSet::Dense(new_dense); + changed + } + } + } + + HybridBitSet::Dense(self_dense) => self_dense.union(other), + } + } + + fn subtract(&mut self, other: &HybridBitSet) -> bool { + // FIXME(willcrichton): should there be an optimized sparse / dense version? + match self { + HybridBitSet::Sparse(self_sparse) => self_sparse.subtract(other), + HybridBitSet::Dense(self_dense) => self_dense.subtract(other), + } + } + + fn intersect(&mut self, other: &HybridBitSet) -> bool { + // FIXME(willcrichton): should there be an optimized sparse / dense version? + match self { + HybridBitSet::Sparse(self_sparse) => self_sparse.intersect(other), + HybridBitSet::Dense(self_dense) => { + as BitRelations>>::intersect(self_dense, other) + } + } } } @@ -441,28 +593,8 @@ impl SparseBitSet { fn iter(&self) -> slice::Iter<'_, T> { self.elems.iter() } -} -impl UnionIntoBitSet for SparseBitSet { - fn union_into(&self, other: &mut BitSet) -> bool { - assert_eq!(self.domain_size, other.domain_size); - let mut changed = false; - for elem in self.iter() { - changed |= other.insert(*elem); - } - changed - } -} - -impl SubtractFromBitSet for SparseBitSet { - fn subtract_from(&self, other: &mut BitSet) -> bool { - assert_eq!(self.domain_size, other.domain_size); - let mut changed = false; - for elem in self.iter() { - changed |= other.remove(*elem); - } - changed - } + bit_relations_inherent_impls! {} } /// A fixed-size bitset type with a hybrid representation: sparse when there @@ -579,48 +711,6 @@ impl HybridBitSet { } } - pub fn union(&mut self, other: &HybridBitSet) -> bool { - match self { - HybridBitSet::Sparse(self_sparse) => { - match other { - HybridBitSet::Sparse(other_sparse) => { - // Both sets are sparse. Add the elements in - // `other_sparse` to `self` one at a time. This - // may or may not cause `self` to be densified. - assert_eq!(self.domain_size(), other.domain_size()); - let mut changed = false; - for elem in other_sparse.iter() { - changed |= self.insert(*elem); - } - changed - } - HybridBitSet::Dense(other_dense) => { - // `self` is sparse and `other` is dense. To - // merge them, we have two available strategies: - // * Densify `self` then merge other - // * Clone other then integrate bits from `self` - // The second strategy requires dedicated method - // since the usual `union` returns the wrong - // result. In the dedicated case the computation - // is slightly faster if the bits of the sparse - // bitset map to only few words of the dense - // representation, i.e. indices are near each - // other. - // - // Benchmarking seems to suggest that the second - // option is worth it. - let mut new_dense = other_dense.clone(); - let changed = new_dense.reverse_union_sparse(self_sparse); - *self = HybridBitSet::Dense(new_dense); - changed - } - } - } - - HybridBitSet::Dense(self_dense) => self_dense.union(other), - } - } - /// Converts to a dense set, consuming itself in the process. pub fn to_dense(self) -> BitSet { match self { @@ -635,24 +725,8 @@ impl HybridBitSet { HybridBitSet::Dense(dense) => HybridIter::Dense(dense.iter()), } } -} -impl UnionIntoBitSet for HybridBitSet { - fn union_into(&self, other: &mut BitSet) -> bool { - match self { - HybridBitSet::Sparse(sparse) => sparse.union_into(other), - HybridBitSet::Dense(dense) => dense.union_into(other), - } - } -} - -impl SubtractFromBitSet for HybridBitSet { - fn subtract_from(&self, other: &mut BitSet) -> bool { - match self { - HybridBitSet::Sparse(sparse) => sparse.subtract_from(other), - HybridBitSet::Dense(dense) => dense.subtract_from(other), - } - } + bit_relations_inherent_impls! {} } pub enum HybridIter<'a, T: Idx> { @@ -974,6 +1048,19 @@ impl SparseBitMatrix { self.ensure_row(row).insert(column) } + pub fn remove(&mut self, row: R, column: C) -> bool { + match self.rows.get_mut(row) { + Some(Some(row)) => row.remove(column), + _ => false, + } + } + + pub fn clear(&mut self, row: R) { + if let Some(Some(row)) = self.rows.get_mut(row) { + row.clear(); + } + } + /// Do the bits from `row` contain `column`? Put another way, is /// the matrix cell at `(row, column)` true? Put yet another way, /// if the matrix represents (transitive) reachability, can @@ -1002,11 +1089,6 @@ impl SparseBitMatrix { } } - /// Union a row, `from`, into the `into` row. - pub fn union_into_row(&mut self, into: R, from: &HybridBitSet) -> bool { - self.ensure_row(into).union(from) - } - /// Insert all bits in the given row. pub fn insert_all_into_row(&mut self, row: R) { self.ensure_row(row).insert_all(); @@ -1025,6 +1107,33 @@ impl SparseBitMatrix { pub fn row(&self, row: R) -> Option<&HybridBitSet> { if let Some(Some(row)) = self.rows.get(row) { Some(row) } else { None } } + + pub fn intersect_row(&mut self, row: R, set: &Set) -> bool + where + HybridBitSet: BitRelations, + { + match self.rows.get_mut(row) { + Some(Some(row)) => row.intersect(set), + _ => false, + } + } + + pub fn subtract_row(&mut self, row: R, set: &Set) -> bool + where + HybridBitSet: BitRelations, + { + match self.rows.get_mut(row) { + Some(Some(row)) => row.subtract(set), + _ => false, + } + } + + pub fn union_row(&mut self, row: R, set: &Set) -> bool + where + HybridBitSet: BitRelations, + { + self.ensure_row(row).union(set) + } } #[inline] diff --git a/compiler/rustc_mir/src/borrow_check/region_infer/values.rs b/compiler/rustc_mir/src/borrow_check/region_infer/values.rs index f247d07e1f05e..2864abde0022c 100644 --- a/compiler/rustc_mir/src/borrow_check/region_infer/values.rs +++ b/compiler/rustc_mir/src/borrow_check/region_infer/values.rs @@ -160,7 +160,7 @@ impl LivenessValues { /// region. Returns whether any of them are newly added. crate fn add_elements(&mut self, row: N, locations: &HybridBitSet) -> bool { debug!("LivenessValues::add_elements(row={:?}, locations={:?})", row, locations); - self.points.union_into_row(row, locations) + self.points.union_row(row, locations) } /// Adds all the control-flow points to the values for `r`. @@ -294,7 +294,7 @@ impl RegionValues { /// the region `to` in `self`. crate fn merge_liveness(&mut self, to: N, from: M, values: &LivenessValues) { if let Some(set) = values.points.row(from) { - self.points.union_into_row(to, set); + self.points.union_row(to, set); } } diff --git a/compiler/rustc_mir/src/transform/generator.rs b/compiler/rustc_mir/src/transform/generator.rs index 963f93a1acec1..acdaa5b456857 100644 --- a/compiler/rustc_mir/src/transform/generator.rs +++ b/compiler/rustc_mir/src/transform/generator.rs @@ -626,7 +626,7 @@ fn compute_storage_conflicts( // Locals that are always live or ones that need to be stored across // suspension points are not eligible for overlap. let mut ineligible_locals = always_live_locals.into_inner(); - ineligible_locals.intersect(saved_locals); + ineligible_locals.intersect(&**saved_locals); // Compute the storage conflicts for all eligible locals. let mut visitor = StorageConflictVisitor { @@ -701,7 +701,7 @@ impl<'body, 'tcx, 's> StorageConflictVisitor<'body, 'tcx, 's> { } let mut eligible_storage_live = flow_state.clone(); - eligible_storage_live.intersect(&self.saved_locals); + eligible_storage_live.intersect(&**self.saved_locals); for local in eligible_storage_live.iter() { self.local_conflicts.union_row_with(&eligible_storage_live, local); From 6cf3786ba48198a54f447274d4928e538b9159bd Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 24 Aug 2021 18:14:39 -0700 Subject: [PATCH 03/19] Fix HybridBitSet port issue --- compiler/rustc_index/src/bit_set.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 0b5745072ad11..bfe2082e756a9 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -247,10 +247,13 @@ impl BitRelations> for BitSet { } } -fn sequential_update(mut f: impl FnMut(T) -> bool, it: impl Iterator) -> bool { +fn sequential_update( + mut self_update: impl FnMut(T) -> bool, + it: impl Iterator, +) -> bool { let mut changed = false; for elem in it { - changed |= f(elem); + changed |= self_update(elem); } changed } @@ -342,7 +345,17 @@ impl BitRelations> for HybridBitSet { match self { HybridBitSet::Sparse(self_sparse) => { match other { - HybridBitSet::Sparse(other_sparse) => self_sparse.union(other_sparse), + HybridBitSet::Sparse(other_sparse) => { + // Both sets are sparse. Add the elements in + // `other_sparse` to `self` one at a time. This + // may or may not cause `self` to be densified. + assert_eq!(self.domain_size(), other.domain_size()); + let mut changed = false; + for elem in other_sparse.iter() { + changed |= self.insert(*elem); + } + changed + } HybridBitSet::Dense(other_dense) => { // `self` is sparse and `other` is dense. To From 415d5e860f2ef823564a8f0e704d4f35e5d07db8 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 25 Aug 2021 15:03:09 -0700 Subject: [PATCH 04/19] Remove BitRelations impls for SparseBitSet, add optimizations --- compiler/rustc_index/src/bit_set.rs | 92 +++++++++-------------------- 1 file changed, 29 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index bfe2082e756a9..46b1c554a61df 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -262,79 +262,42 @@ fn sparse_intersect( set: &mut SparseBitSet, other_contains: impl Fn(&T) -> bool, ) -> bool { - let mut changed = false; - for i in (0..set.len()).rev() { - if !other_contains(&set.elems[i]) { - set.elems.remove(i); - changed = true; - } - } - changed -} - -impl BitRelations> for BitSet { - fn union(&mut self, other: &SparseBitSet) -> bool { - sequential_update(|elem| self.insert(elem), other.iter().cloned()) - } - - fn subtract(&mut self, other: &SparseBitSet) -> bool { - sequential_update(|elem| self.remove(elem), other.iter().cloned()) - } - - fn intersect(&mut self, other: &SparseBitSet) -> bool { - self.intersect(&other.to_dense()) - } + let size = set.elems.len(); + set.elems.retain(|elem| other_contains(elem)); + set.elems.len() != size } -impl BitRelations> for SparseBitSet { - fn union(&mut self, other: &BitSet) -> bool { - sequential_update(|elem| self.insert(elem), other.iter()) - } - - fn subtract(&mut self, other: &BitSet) -> bool { - sequential_update(|elem| self.remove(elem), other.iter()) - } - - fn intersect(&mut self, other: &BitSet) -> bool { - sparse_intersect(self, |el| other.contains(*el)) - } -} - -impl BitRelations> for SparseBitSet { - fn union(&mut self, other: &SparseBitSet) -> bool { - sequential_update(|elem| self.insert(elem), other.iter().cloned()) - } - - fn subtract(&mut self, other: &SparseBitSet) -> bool { - sequential_update(|elem| self.insert(elem), other.iter().cloned()) - } - - fn intersect(&mut self, other: &SparseBitSet) -> bool { - sparse_intersect(self, |el| other.contains(*el)) - } -} - -impl BitRelations> for S -where - S: BitRelations> + BitRelations>, -{ +impl BitRelations> for BitSet { fn union(&mut self, other: &HybridBitSet) -> bool { + assert_eq!(self.domain_size, other.domain_size()); match other { - HybridBitSet::Sparse(sparse) => self.union(sparse), + HybridBitSet::Sparse(sparse) => { + sequential_update(|elem| self.insert(elem), sparse.iter().cloned()) + } HybridBitSet::Dense(dense) => self.union(dense), } } fn subtract(&mut self, other: &HybridBitSet) -> bool { + assert_eq!(self.domain_size, other.domain_size()); match other { - HybridBitSet::Sparse(sparse) => self.subtract(sparse), + HybridBitSet::Sparse(sparse) => { + sequential_update(|elem| self.remove(elem), sparse.iter().cloned()) + } HybridBitSet::Dense(dense) => self.subtract(dense), } } fn intersect(&mut self, other: &HybridBitSet) -> bool { + assert_eq!(self.domain_size, other.domain_size()); match other { - HybridBitSet::Sparse(sparse) => self.intersect(sparse), + HybridBitSet::Sparse(sparse) => { + let n = self.count(); + let mut sparse_copy = sparse.clone(); + sparse_intersect(&mut sparse_copy, |el| !self.contains(*el)); + *self = sparse_copy.to_dense(); + self.count() != n + } HybridBitSet::Dense(dense) => self.intersect(dense), } } @@ -342,6 +305,7 @@ where impl BitRelations> for HybridBitSet { fn union(&mut self, other: &HybridBitSet) -> bool { + assert_eq!(self.domain_size(), other.domain_size()); match self { HybridBitSet::Sparse(self_sparse) => { match other { @@ -385,20 +349,22 @@ impl BitRelations> for HybridBitSet { } fn subtract(&mut self, other: &HybridBitSet) -> bool { - // FIXME(willcrichton): should there be an optimized sparse / dense version? + assert_eq!(self.domain_size(), other.domain_size()); match self { - HybridBitSet::Sparse(self_sparse) => self_sparse.subtract(other), + HybridBitSet::Sparse(self_sparse) => { + sequential_update(|elem| self_sparse.remove(elem), other.iter()) + } HybridBitSet::Dense(self_dense) => self_dense.subtract(other), } } fn intersect(&mut self, other: &HybridBitSet) -> bool { - // FIXME(willcrichton): should there be an optimized sparse / dense version? + assert_eq!(self.domain_size(), other.domain_size()); match self { - HybridBitSet::Sparse(self_sparse) => self_sparse.intersect(other), - HybridBitSet::Dense(self_dense) => { - as BitRelations>>::intersect(self_dense, other) + HybridBitSet::Sparse(self_sparse) => { + sparse_intersect(self_sparse, |elem| other.contains(*elem)) } + HybridBitSet::Dense(self_dense) => self_dense.intersect(other), } } } From 2110ac303ed53b77806c06c963b8fa086f87e909 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 25 Aug 2021 15:10:33 -0700 Subject: [PATCH 05/19] Add optimized sparse-hybrid / dense-hybrid intersect --- compiler/rustc_index/src/bit_set.rs | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 46b1c554a61df..8793d56792a7e 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -267,6 +267,16 @@ fn sparse_intersect( set.elems.len() != size } +fn dense_sparse_intersect( + dense: &BitSet, + sparse: &SparseBitSet, +) -> (SparseBitSet, bool) { + let n = dense.count(); + let mut sparse_copy = sparse.clone(); + sparse_intersect(&mut sparse_copy, |el| !dense.contains(*el)); + (sparse_copy, dense.count() != n) +} + impl BitRelations> for BitSet { fn union(&mut self, other: &HybridBitSet) -> bool { assert_eq!(self.domain_size, other.domain_size()); @@ -292,11 +302,9 @@ impl BitRelations> for BitSet { assert_eq!(self.domain_size, other.domain_size()); match other { HybridBitSet::Sparse(sparse) => { - let n = self.count(); - let mut sparse_copy = sparse.clone(); - sparse_intersect(&mut sparse_copy, |el| !self.contains(*el)); - *self = sparse_copy.to_dense(); - self.count() != n + let (updated, changed) = dense_sparse_intersect(self, sparse); + *self = updated.to_dense(); + changed } HybridBitSet::Dense(dense) => self.intersect(dense), } @@ -364,7 +372,14 @@ impl BitRelations> for HybridBitSet { HybridBitSet::Sparse(self_sparse) => { sparse_intersect(self_sparse, |elem| other.contains(*elem)) } - HybridBitSet::Dense(self_dense) => self_dense.intersect(other), + HybridBitSet::Dense(self_dense) => match other { + HybridBitSet::Sparse(other_sparse) => { + let (updated, changed) = dense_sparse_intersect(self_dense, other_sparse); + *self = HybridBitSet::Sparse(updated); + changed + } + HybridBitSet::Dense(other_dense) => self_dense.intersect(other_dense), + }, } } } From 800d6531a97268dbd371d879c9e9bd0a6368f2f4 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 25 Aug 2021 22:54:26 -0700 Subject: [PATCH 06/19] Small fixes --- compiler/rustc_index/src/bit_set.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 8793d56792a7e..230a551f3c5f8 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -271,10 +271,10 @@ fn dense_sparse_intersect( dense: &BitSet, sparse: &SparseBitSet, ) -> (SparseBitSet, bool) { - let n = dense.count(); let mut sparse_copy = sparse.clone(); sparse_intersect(&mut sparse_copy, |el| !dense.contains(*el)); - (sparse_copy, dense.count() != n) + let n = sparse_copy.len(); + (sparse_copy, n != dense.count()) } impl BitRelations> for BitSet { @@ -303,7 +303,10 @@ impl BitRelations> for BitSet { match other { HybridBitSet::Sparse(sparse) => { let (updated, changed) = dense_sparse_intersect(self, sparse); - *self = updated.to_dense(); + self.clear(); + for elem in updated.iter() { + self.insert(*elem); + } changed } HybridBitSet::Dense(dense) => self.intersect(dense), From 1c1603e0b50245e4aedca47d9a46904ac7dd5255 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 25 Aug 2021 23:15:21 -0700 Subject: [PATCH 07/19] Add unit tests for BitSet intersect/subtract --- compiler/rustc_index/src/bit_set/tests.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_index/src/bit_set/tests.rs b/compiler/rustc_index/src/bit_set/tests.rs index c11b98e77aa58..352a0c7a1b4fa 100644 --- a/compiler/rustc_index/src/bit_set/tests.rs +++ b/compiler/rustc_index/src/bit_set/tests.rs @@ -110,12 +110,23 @@ fn hybrid_bitset() { assert!(hybrid.superset(&sparse01358) && sparse01358.superset(&hybrid)); assert!(!dense10.union(&sparse01358)); assert!(!dense256.union(&dense10)); - let mut dense = dense10; + + assert!(dense10.clone().intersect(&sparse01358)); + assert!(!sparse01358.clone().intersect(&dense10)); + assert!(dense10.clone().subtract(&sparse01358)); + assert!(sparse01358.clone().subtract(&dense10)); + + let mut dense = dense10.clone(); assert!(dense.union(&dense256)); assert!(dense.superset(&dense256) && dense256.superset(&dense)); assert!(hybrid.union(&dense256)); assert!(hybrid.superset(&dense256) && dense256.superset(&hybrid)); + assert!(!dense10.clone().intersect(&dense256)); + assert!(dense256.clone().intersect(&dense10)); + assert!(dense10.clone().subtract(&dense256)); + assert!(dense256.clone().subtract(&dense10)); + assert_eq!(dense256.iter().count(), 256); let mut dense0 = dense256; for i in 0..256 { From d73a169f93748e963be068201b5c4eee1fe6c982 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 11:39:13 -0700 Subject: [PATCH 08/19] Fix sparse intersect bug, add more sparse / dense tests --- compiler/rustc_index/src/bit_set.rs | 2 +- compiler/rustc_index/src/bit_set/tests.rs | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 230a551f3c5f8..989d56b0528e0 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -272,7 +272,7 @@ fn dense_sparse_intersect( sparse: &SparseBitSet, ) -> (SparseBitSet, bool) { let mut sparse_copy = sparse.clone(); - sparse_intersect(&mut sparse_copy, |el| !dense.contains(*el)); + sparse_intersect(&mut sparse_copy, |el| dense.contains(*el)); let n = sparse_copy.len(); (sparse_copy, n != dense.count()) } diff --git a/compiler/rustc_index/src/bit_set/tests.rs b/compiler/rustc_index/src/bit_set/tests.rs index 352a0c7a1b4fa..6f633f3b9cbbe 100644 --- a/compiler/rustc_index/src/bit_set/tests.rs +++ b/compiler/rustc_index/src/bit_set/tests.rs @@ -108,14 +108,25 @@ fn hybrid_bitset() { assert!(!sparse01358.union(&hybrid)); // no change assert!(hybrid.union(&sparse01358)); assert!(hybrid.superset(&sparse01358) && sparse01358.superset(&hybrid)); - assert!(!dense10.union(&sparse01358)); assert!(!dense256.union(&dense10)); + // dense / sparse where dense superset sparse + assert!(!dense10.clone().union(&sparse01358)); + assert!(sparse01358.clone().union(&dense10)); assert!(dense10.clone().intersect(&sparse01358)); assert!(!sparse01358.clone().intersect(&dense10)); assert!(dense10.clone().subtract(&sparse01358)); assert!(sparse01358.clone().subtract(&dense10)); + // dense / sparse where sparse superset dense + let dense038 = sparse038.to_dense(); + assert!(!sparse0381.clone().union(&dense038)); + assert!(dense038.clone().union(&sparse0381)); + assert!(sparse0381.clone().intersect(&dense038)); + assert!(!dense038.clone().intersect(&sparse0381)); + assert!(sparse0381.clone().subtract(&dense038)); + assert!(dense038.clone().subtract(&sparse0381)); + let mut dense = dense10.clone(); assert!(dense.union(&dense256)); assert!(dense.superset(&dense256) && dense256.superset(&dense)); From ce37f0a35559fb5396881f3d9a547f161b1e822d Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 11:45:25 -0700 Subject: [PATCH 09/19] Add comments --- compiler/rustc_index/src/bit_set.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 989d56b0528e0..610421f8060a4 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -247,6 +247,8 @@ impl BitRelations> for BitSet { } } +// Applies a function to mutate a bitset, and returns true if any +// of the applications return true fn sequential_update( mut self_update: impl FnMut(T) -> bool, it: impl Iterator, @@ -258,6 +260,8 @@ fn sequential_update( changed } +// Optimization of intersection for SparseBitSet that's generic +// over the RHS fn sparse_intersect( set: &mut SparseBitSet, other_contains: impl Fn(&T) -> bool, @@ -267,6 +271,10 @@ fn sparse_intersect( set.elems.len() != size } +// Optimization of dense/sparse intersection. The resulting set is +// guaranteed to be at most the size of the sparse set, and hence can be +// represented as a sparse set. Therefore the sparse set is copied and filtered, +// then returned as the new set. fn dense_sparse_intersect( dense: &BitSet, sparse: &SparseBitSet, @@ -303,6 +311,10 @@ impl BitRelations> for BitSet { match other { HybridBitSet::Sparse(sparse) => { let (updated, changed) = dense_sparse_intersect(self, sparse); + + // We can't directly assign the BitSet to the SparseBitSet, and + // doing `*self = updated.to_dense()` would cause a drop / reallocation. Instead, + // the BitSet is cleared and `updated` is copied into `self`. self.clear(); for elem in updated.iter() { self.insert(*elem); From 8767b00d67a561af5a4e97d4ccbc8f53f931df2a Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 11:46:00 -0700 Subject: [PATCH 10/19] Formatting --- compiler/rustc_index/src/bit_set.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 610421f8060a4..f5768198f3466 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -274,7 +274,7 @@ fn sparse_intersect( // Optimization of dense/sparse intersection. The resulting set is // guaranteed to be at most the size of the sparse set, and hence can be // represented as a sparse set. Therefore the sparse set is copied and filtered, -// then returned as the new set. +// then returned as the new set. fn dense_sparse_intersect( dense: &BitSet, sparse: &SparseBitSet, @@ -312,7 +312,7 @@ impl BitRelations> for BitSet { HybridBitSet::Sparse(sparse) => { let (updated, changed) = dense_sparse_intersect(self, sparse); - // We can't directly assign the BitSet to the SparseBitSet, and + // We can't directly assign the BitSet to the SparseBitSet, and // doing `*self = updated.to_dense()` would cause a drop / reallocation. Instead, // the BitSet is cleared and `updated` is copied into `self`. self.clear(); From e854027c12f88d9d28c5a016ee8b6a6cb89983cc Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 11:46:57 -0700 Subject: [PATCH 11/19] Compilation failure in tests --- compiler/rustc_index/src/bit_set/tests.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_index/src/bit_set/tests.rs b/compiler/rustc_index/src/bit_set/tests.rs index 6f633f3b9cbbe..7524c209aba16 100644 --- a/compiler/rustc_index/src/bit_set/tests.rs +++ b/compiler/rustc_index/src/bit_set/tests.rs @@ -120,12 +120,12 @@ fn hybrid_bitset() { // dense / sparse where sparse superset dense let dense038 = sparse038.to_dense(); - assert!(!sparse0381.clone().union(&dense038)); - assert!(dense038.clone().union(&sparse0381)); - assert!(sparse0381.clone().intersect(&dense038)); - assert!(!dense038.clone().intersect(&sparse0381)); - assert!(sparse0381.clone().subtract(&dense038)); - assert!(dense038.clone().subtract(&sparse0381)); + assert!(!sparse01358.clone().union(&dense038)); + assert!(dense038.clone().union(&sparse01358)); + assert!(sparse01358.clone().intersect(&dense038)); + assert!(!dense038.clone().intersect(&sparse01358)); + assert!(sparse01358.clone().subtract(&dense038)); + assert!(dense038.clone().subtract(&sparse01358)); let mut dense = dense10.clone(); assert!(dense.union(&dense256)); From 953d685ea119abc28c38738efe8961adc54e7b08 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 12:12:29 -0700 Subject: [PATCH 12/19] Add remaining impl for hybrid X dense --- compiler/rustc_index/src/bit_set.rs | 72 ++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index f5768198f3466..1bb323cb2c43c 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -230,6 +230,7 @@ impl BitSet { bit_relations_inherent_impls! {} } +// dense REL dense impl BitRelations> for BitSet { fn union(&mut self, other: &BitSet) -> bool { assert_eq!(self.domain_size, other.domain_size); @@ -285,6 +286,53 @@ fn dense_sparse_intersect( (sparse_copy, n != dense.count()) } +// hybrid REL dense +impl BitRelations> for HybridBitSet { + fn union(&mut self, other: &BitSet) -> bool { + match self { + HybridBitSet::Sparse(sparse) => { + // `self` is sparse and `other` is dense. To + // merge them, we have two available strategies: + // * Densify `self` then merge other + // * Clone other then integrate bits from `self` + // The second strategy requires dedicated method + // since the usual `union` returns the wrong + // result. In the dedicated case the computation + // is slightly faster if the bits of the sparse + // bitset map to only few words of the dense + // representation, i.e. indices are near each + // other. + // + // Benchmarking seems to suggest that the second + // option is worth it. + let mut new_dense = other.clone(); + let changed = new_dense.reverse_union_sparse(sparse); + *self = HybridBitSet::Dense(new_dense); + changed + } + + HybridBitSet::Dense(dense) => dense.union(other), + } + } + + fn subtract(&mut self, other: &BitSet) -> bool { + match self { + HybridBitSet::Sparse(sparse) => { + sequential_update(|elem| sparse.remove(elem), other.iter()) + } + HybridBitSet::Dense(dense) => dense.subtract(other), + } + } + + fn intersect(&mut self, other: &BitSet) -> bool { + match self { + HybridBitSet::Sparse(sparse) => sparse_intersect(sparse, |elem| other.contains(*elem)), + HybridBitSet::Dense(dense) => dense.intersect(other), + } + } +} + +// dense REL hybrid impl BitRelations> for BitSet { fn union(&mut self, other: &HybridBitSet) -> bool { assert_eq!(self.domain_size, other.domain_size()); @@ -326,13 +374,14 @@ impl BitRelations> for BitSet { } } +// hybrid REL hybrid impl BitRelations> for HybridBitSet { fn union(&mut self, other: &HybridBitSet) -> bool { assert_eq!(self.domain_size(), other.domain_size()); match self { HybridBitSet::Sparse(self_sparse) => { match other { - HybridBitSet::Sparse(other_sparse) => { + HybridBitSet::Sparse(_) => { // Both sets are sparse. Add the elements in // `other_sparse` to `self` one at a time. This // may or may not cause `self` to be densified. @@ -344,26 +393,7 @@ impl BitRelations> for HybridBitSet { changed } - HybridBitSet::Dense(other_dense) => { - // `self` is sparse and `other` is dense. To - // merge them, we have two available strategies: - // * Densify `self` then merge other - // * Clone other then integrate bits from `self` - // The second strategy requires dedicated method - // since the usual `union` returns the wrong - // result. In the dedicated case the computation - // is slightly faster if the bits of the sparse - // bitset map to only few words of the dense - // representation, i.e. indices are near each - // other. - // - // Benchmarking seems to suggest that the second - // option is worth it. - let mut new_dense = other_dense.clone(); - let changed = new_dense.reverse_union_sparse(self_sparse); - *self = HybridBitSet::Dense(new_dense); - changed - } + HybridBitSet::Dense(other_dense) => self.union(other_dense), } } From acba31c33378c961b37c5ea66c23675c101cb2e8 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 12:14:37 -0700 Subject: [PATCH 13/19] Typo --- compiler/rustc_index/src/bit_set.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 1bb323cb2c43c..fff0b3df7f91c 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -379,9 +379,9 @@ impl BitRelations> for HybridBitSet { fn union(&mut self, other: &HybridBitSet) -> bool { assert_eq!(self.domain_size(), other.domain_size()); match self { - HybridBitSet::Sparse(self_sparse) => { + HybridBitSet::Sparse(_) => { match other { - HybridBitSet::Sparse(_) => { + HybridBitSet::Sparse(other_sparse) => { // Both sets are sparse. Add the elements in // `other_sparse` to `self` one at a time. This // may or may not cause `self` to be densified. From 7e148b0cef3eccf06414c47841a525c7cc096f28 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 12:26:08 -0700 Subject: [PATCH 14/19] Compile failure --- compiler/rustc_index/src/bit_set/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_index/src/bit_set/tests.rs b/compiler/rustc_index/src/bit_set/tests.rs index 7524c209aba16..6fda7a03f8822 100644 --- a/compiler/rustc_index/src/bit_set/tests.rs +++ b/compiler/rustc_index/src/bit_set/tests.rs @@ -104,7 +104,7 @@ fn hybrid_bitset() { assert!(dense10.superset(&dense10)); // dense + dense (self) assert!(dense256.superset(&dense10)); // dense + dense - let mut hybrid = sparse038; + let mut hybrid = sparse038.clone(); assert!(!sparse01358.union(&hybrid)); // no change assert!(hybrid.union(&sparse01358)); assert!(hybrid.superset(&sparse01358) && sparse01358.superset(&hybrid)); From 2166c6db438911590d2ab70964010217e9697a09 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 12:46:59 -0700 Subject: [PATCH 15/19] Add comments and unit tests for new SparseBitMatrix methods --- compiler/rustc_index/src/bit_set.rs | 19 +++++++ compiler/rustc_index/src/bit_set/tests.rs | 66 +++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index fff0b3df7f91c..fad3b95cf43a9 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -1087,6 +1087,11 @@ impl SparseBitMatrix { self.ensure_row(row).insert(column) } + /// Sets the cell at `(row, column)` to false. Put another way, delete + /// `column` from the bitset for `row`. Has no effect if `row` does not + /// exist. + /// + /// Returns `true` if this changed the matrix. pub fn remove(&mut self, row: R, column: C) -> bool { match self.rows.get_mut(row) { Some(Some(row)) => row.remove(column), @@ -1094,6 +1099,8 @@ impl SparseBitMatrix { } } + /// Sets all columns at `row` to false. Has no effect if `row` does + /// not exist. pub fn clear(&mut self, row: R) { if let Some(Some(row)) = self.rows.get_mut(row) { row.clear(); @@ -1147,6 +1154,10 @@ impl SparseBitMatrix { if let Some(Some(row)) = self.rows.get(row) { Some(row) } else { None } } + /// Interescts `row` with `set`. `set` can be either `BitSet` or + /// `HybridBitSet`. Has no effect if `row` does not exist. + /// + /// Returns true if the row was changed. pub fn intersect_row(&mut self, row: R, set: &Set) -> bool where HybridBitSet: BitRelations, @@ -1157,6 +1168,10 @@ impl SparseBitMatrix { } } + /// Subtracts `set from `row`. `set` can be either `BitSet` or + /// `HybridBitSet`. Has no effect if `row` does not exist. + /// + /// Returns true if the row was changed. pub fn subtract_row(&mut self, row: R, set: &Set) -> bool where HybridBitSet: BitRelations, @@ -1167,6 +1182,10 @@ impl SparseBitMatrix { } } + /// Unions `row` with `set`. `set` can be either `BitSet` or + /// `HybridBitSet`. + /// + /// Returns true if the row was changed. pub fn union_row(&mut self, row: R, set: &Set) -> bool where HybridBitSet: BitRelations, diff --git a/compiler/rustc_index/src/bit_set/tests.rs b/compiler/rustc_index/src/bit_set/tests.rs index 6fda7a03f8822..6ea249a0b693a 100644 --- a/compiler/rustc_index/src/bit_set/tests.rs +++ b/compiler/rustc_index/src/bit_set/tests.rs @@ -304,6 +304,72 @@ fn sparse_matrix_iter() { assert!(iter.next().is_none()); } +#[test] +fn sparse_matrix_operations() { + let mut matrix: SparseBitMatrix = SparseBitMatrix::new(100); + matrix.insert(3, 22); + matrix.insert(3, 75); + matrix.insert(2, 99); + matrix.insert(4, 0); + + let mut disjoint: HybridBitSet = HybridBitSet::new_empty(100); + disjoint.insert(33); + + let mut superset = HybridBitSet::new_empty(100); + superset.insert(22); + superset.insert(75); + superset.insert(33); + + let mut subset = HybridBitSet::new_empty(100); + subset.insert(22); + + // SparseBitMatrix::remove + { + let mut matrix = matrix.clone(); + matrix.remove(3, 22); + assert!(!matrix.row(3).unwrap().contains(22)); + matrix.remove(0, 0); + assert!(matrix.row(0).is_none()); + } + + // SparseBitMatrix::clear + { + let mut matrix = matrix.clone(); + matrix.clear(3); + assert!(!matrix.row(3).unwrap().contains(75)); + matrix.clear(0); + assert!(matrix.row(0).is_none()); + } + + // SparseBitMatrix::intersect_row + { + let mut matrix = matrix.clone(); + assert!(!matrix.intersect_row(2, &superset)); + assert!(matrix.intersect_row(2, &subset)); + matrix.intersect_row(0, &disjoint); + assert!(matrix.row(0).is_none()); + } + + // SparseBitMatrix::subtract_row + { + let mut matrix = matrix.clone(); + assert!(!matrix.subtract_row(2, &disjoint)); + assert!(matrix.subtract_row(2, &subset)); + assert!(matrix.subtract_row(2, &superset)); + matrix.intersect_row(0, &disjoint); + assert!(matrix.row(0).is_none()); + } + + // SparseBitMatrix::union_row + { + let mut matrix = matrix.clone(); + assert!(!matrix.union_row(2, &subset)); + assert!(matrix.union_row(2, &disjoint)); + matrix.union_row(0, &disjoint); + assert!(matrix.row(0).is_some()); + } +} + /// Merge dense hybrid set into empty sparse hybrid set. #[bench] fn union_hybrid_sparse_empty_to_dense(b: &mut Bencher) { From 8d9e4f98e142dd7a6fd61deac46eddf8bb4a45be Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 13:09:39 -0700 Subject: [PATCH 16/19] Fix failing test --- compiler/rustc_index/src/bit_set/tests.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_index/src/bit_set/tests.rs b/compiler/rustc_index/src/bit_set/tests.rs index 6ea249a0b693a..5ff53211bc7e1 100644 --- a/compiler/rustc_index/src/bit_set/tests.rs +++ b/compiler/rustc_index/src/bit_set/tests.rs @@ -344,8 +344,8 @@ fn sparse_matrix_operations() { // SparseBitMatrix::intersect_row { let mut matrix = matrix.clone(); - assert!(!matrix.intersect_row(2, &superset)); - assert!(matrix.intersect_row(2, &subset)); + assert!(!matrix.intersect_row(3, &superset)); + assert!(matrix.intersect_row(3, &subset)); matrix.intersect_row(0, &disjoint); assert!(matrix.row(0).is_none()); } @@ -353,9 +353,9 @@ fn sparse_matrix_operations() { // SparseBitMatrix::subtract_row { let mut matrix = matrix.clone(); - assert!(!matrix.subtract_row(2, &disjoint)); - assert!(matrix.subtract_row(2, &subset)); - assert!(matrix.subtract_row(2, &superset)); + assert!(!matrix.subtract_row(3, &disjoint)); + assert!(matrix.subtract_row(3, &subset)); + assert!(matrix.subtract_row(3, &superset)); matrix.intersect_row(0, &disjoint); assert!(matrix.row(0).is_none()); } @@ -363,8 +363,8 @@ fn sparse_matrix_operations() { // SparseBitMatrix::union_row { let mut matrix = matrix.clone(); - assert!(!matrix.union_row(2, &subset)); - assert!(matrix.union_row(2, &disjoint)); + assert!(!matrix.union_row(3, &subset)); + assert!(matrix.union_row(3, &disjoint)); matrix.union_row(0, &disjoint); assert!(matrix.row(0).is_some()); } From c7357270b8b93ef23f6295abecf8ff27948881c3 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Thu, 26 Aug 2021 13:23:24 -0700 Subject: [PATCH 17/19] Formatting --- compiler/rustc_index/src/bit_set/tests.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_index/src/bit_set/tests.rs b/compiler/rustc_index/src/bit_set/tests.rs index 5ff53211bc7e1..aebc6d0ddd84c 100644 --- a/compiler/rustc_index/src/bit_set/tests.rs +++ b/compiler/rustc_index/src/bit_set/tests.rs @@ -350,24 +350,24 @@ fn sparse_matrix_operations() { assert!(matrix.row(0).is_none()); } - // SparseBitMatrix::subtract_row - { + // SparseBitMatrix::subtract_row + { let mut matrix = matrix.clone(); assert!(!matrix.subtract_row(3, &disjoint)); assert!(matrix.subtract_row(3, &subset)); assert!(matrix.subtract_row(3, &superset)); matrix.intersect_row(0, &disjoint); assert!(matrix.row(0).is_none()); - } + } - // SparseBitMatrix::union_row - { + // SparseBitMatrix::union_row + { let mut matrix = matrix.clone(); assert!(!matrix.union_row(3, &subset)); assert!(matrix.union_row(3, &disjoint)); matrix.union_row(0, &disjoint); assert!(matrix.row(0).is_some()); - } + } } /// Merge dense hybrid set into empty sparse hybrid set. From 86bd551e4c8954330e5e0d71c09c4ecbf514a5b6 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Fri, 27 Aug 2021 11:17:27 -0700 Subject: [PATCH 18/19] Addd missing domain size assertions --- compiler/rustc_index/src/bit_set.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index fad3b95cf43a9..8903ed9d9e2af 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -289,6 +289,7 @@ fn dense_sparse_intersect( // hybrid REL dense impl BitRelations> for HybridBitSet { fn union(&mut self, other: &BitSet) -> bool { + assert_eq!(self.domain_size(), other.domain_size); match self { HybridBitSet::Sparse(sparse) => { // `self` is sparse and `other` is dense. To @@ -316,6 +317,7 @@ impl BitRelations> for HybridBitSet { } fn subtract(&mut self, other: &BitSet) -> bool { + assert_eq!(self.domain_size(), other.domain_size); match self { HybridBitSet::Sparse(sparse) => { sequential_update(|elem| sparse.remove(elem), other.iter()) @@ -325,6 +327,7 @@ impl BitRelations> for HybridBitSet { } fn intersect(&mut self, other: &BitSet) -> bool { + assert_eq!(self.domain_size(), other.domain_size); match self { HybridBitSet::Sparse(sparse) => sparse_intersect(sparse, |elem| other.contains(*elem)), HybridBitSet::Dense(dense) => dense.intersect(other), @@ -385,7 +388,6 @@ impl BitRelations> for HybridBitSet { // Both sets are sparse. Add the elements in // `other_sparse` to `self` one at a time. This // may or may not cause `self` to be densified. - assert_eq!(self.domain_size(), other.domain_size()); let mut changed = false; for elem in other_sparse.iter() { changed |= self.insert(*elem); From e340a0e249c2efc069235c9db47e47c3e136570c Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Fri, 27 Aug 2021 16:21:25 -0700 Subject: [PATCH 19/19] Suggested changes --- compiler/rustc_index/src/bit_set.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_index/src/bit_set.rs b/compiler/rustc_index/src/bit_set.rs index 8903ed9d9e2af..aeb3f9970ab9e 100644 --- a/compiler/rustc_index/src/bit_set.rs +++ b/compiler/rustc_index/src/bit_set.rs @@ -363,7 +363,7 @@ impl BitRelations> for BitSet { HybridBitSet::Sparse(sparse) => { let (updated, changed) = dense_sparse_intersect(self, sparse); - // We can't directly assign the BitSet to the SparseBitSet, and + // We can't directly assign the SparseBitSet to the BitSet, and // doing `*self = updated.to_dense()` would cause a drop / reallocation. Instead, // the BitSet is cleared and `updated` is copied into `self`. self.clear(); @@ -1071,7 +1071,7 @@ impl SparseBitMatrix { Self { num_columns, rows: IndexVec::new() } } - pub fn ensure_row(&mut self, row: R) -> &mut HybridBitSet { + fn ensure_row(&mut self, row: R) -> &mut HybridBitSet { // Instantiate any missing rows up to and including row `row` with an // empty HybridBitSet. self.rows.ensure_contains_elem(row, || None);