From bb549da8f6e050f7cf206488265d300370f5cd7e Mon Sep 17 00:00:00 2001 From: Daniel Lemire Date: Sun, 7 Dec 2025 16:51:27 -0500 Subject: [PATCH 1/8] test: update duplicate many_digits test to use f64 instead of f32 Replace the f32 test case with an f64 equivalent to improve coverage for parsing large digit counts in double-precision floating-point conversion. --- library/coretests/tests/num/dec2flt/parse.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/coretests/tests/num/dec2flt/parse.rs b/library/coretests/tests/num/dec2flt/parse.rs index dccb6b5528d4c..65f1289d531b3 100644 --- a/library/coretests/tests/num/dec2flt/parse.rs +++ b/library/coretests/tests/num/dec2flt/parse.rs @@ -204,8 +204,8 @@ fn many_digits() { "1.175494140627517859246175898662808184331245864732796240031385942718174675986064769972472277004271745681762695312500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e-38" ); assert_float_result_bits_eq!( - 0x7ffffe, - f32, - "1.175494140627517859246175898662808184331245864732796240031385942718174675986064769972472277004271745681762695312500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e-38" + 0x7ffffffffffffe, + f64, + "2.8480945388892171379514712013899006561925415836684281158317117472799232118121416898288331376688117187382156519616555919469923055562697333532151065976805407006156569379201589813489881051279092585294136284133893120582268661680741374638184352784785121471797815387841323043061183721896440504100014432145713413240639315126126485370149254502658324386101932795656342787961697168715161403422599767855490566539165801964388919848888980308652852766053138934233069066651644954964960514065582826296567812754916294792554028205611526494813491373571799099361175786448799007387647056059512705071170383000860694587462575533337769237800154235626508997957207085308694919708914563870082480025309479610576861709891209343110514894825624524442569257542956506767950486391782760620117187500000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e-306" ); } From d602710e5a7813c94bb8cf7d0fb92ae558e1c46a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 13 Dec 2025 20:13:24 +1100 Subject: [PATCH 2/8] Move match-candidate partitioning code into its own module --- .../src/builder/matches/buckets.rs | 349 ++++++++++++++++++ .../src/builder/matches/mod.rs | 81 +--- .../src/builder/matches/test.rs | 263 +------------ 3 files changed, 353 insertions(+), 340 deletions(-) create mode 100644 compiler/rustc_mir_build/src/builder/matches/buckets.rs diff --git a/compiler/rustc_mir_build/src/builder/matches/buckets.rs b/compiler/rustc_mir_build/src/builder/matches/buckets.rs new file mode 100644 index 0000000000000..0643704ed35ad --- /dev/null +++ b/compiler/rustc_mir_build/src/builder/matches/buckets.rs @@ -0,0 +1,349 @@ +use std::cmp::Ordering; + +use rustc_data_structures::fx::FxIndexMap; +use rustc_middle::mir::{BinOp, Place}; +use rustc_middle::span_bug; +use tracing::debug; + +use crate::builder::Builder; +use crate::builder::matches::test::is_switch_ty; +use crate::builder::matches::{Candidate, Test, TestBranch, TestCase, TestKind}; + +impl<'a, 'tcx> Builder<'a, 'tcx> { + /// Given a test, we partition the input candidates into several buckets. + /// If a candidate matches in exactly one of the branches of `test` + /// (and no other branches), we put it into the corresponding bucket. + /// If it could match in more than one of the branches of `test`, the test + /// doesn't usefully apply to it, and we stop partitioning candidates. + /// + /// Importantly, we also **mutate** the branched candidates to remove match pairs + /// that are entailed by the outcome of the test, and add any sub-pairs of the + /// removed pairs. + /// + /// This returns a pair of + /// - the candidates that weren't sorted; + /// - for each possible outcome of the test, the candidates that match in that outcome. + /// + /// For example: + /// ``` + /// # let (x, y, z) = (true, true, true); + /// match (x, y, z) { + /// (true , _ , true ) => true, // (0) + /// (false, false, _ ) => false, // (1) + /// (_ , true , _ ) => true, // (2) + /// (true , _ , false) => false, // (3) + /// } + /// # ; + /// ``` + /// + /// Assume we are testing on `x`. Conceptually, there are 2 overlapping candidate sets: + /// - If the outcome is that `x` is true, candidates {0, 2, 3} are possible + /// - If the outcome is that `x` is false, candidates {1, 2} are possible + /// + /// Following our algorithm: + /// - Candidate 0 is sorted into outcome `x == true` + /// - Candidate 1 is sorted into outcome `x == false` + /// - Candidate 2 remains unsorted, because testing `x` has no effect on it + /// - Candidate 3 remains unsorted, because a previous candidate (2) was unsorted + /// - This helps preserve the illusion that candidates are tested "in order" + /// + /// The sorted candidates are mutated to remove entailed match pairs: + /// - candidate 0 becomes `[z @ true]` since we know that `x` was `true`; + /// - candidate 1 becomes `[y @ false]` since we know that `x` was `false`. + pub(super) fn sort_candidates<'b, 'c>( + &mut self, + match_place: Place<'tcx>, + test: &Test<'tcx>, + mut candidates: &'b mut [&'c mut Candidate<'tcx>], + ) -> ( + &'b mut [&'c mut Candidate<'tcx>], + FxIndexMap, Vec<&'b mut Candidate<'tcx>>>, + ) { + // For each of the possible outcomes, collect vector of candidates that apply if the test + // has that particular outcome. + let mut target_candidates: FxIndexMap<_, Vec<&mut Candidate<'_>>> = Default::default(); + + let total_candidate_count = candidates.len(); + + // Sort the candidates into the appropriate vector in `target_candidates`. Note that at some + // point we may encounter a candidate where the test is not relevant; at that point, we stop + // sorting. + while let Some(candidate) = candidates.first_mut() { + let Some(branch) = + self.sort_candidate(match_place, test, candidate, &target_candidates) + else { + break; + }; + let (candidate, rest) = candidates.split_first_mut().unwrap(); + target_candidates.entry(branch).or_insert_with(Vec::new).push(candidate); + candidates = rest; + } + + // At least the first candidate ought to be tested + assert!( + total_candidate_count > candidates.len(), + "{total_candidate_count}, {candidates:#?}" + ); + debug!("tested_candidates: {}", total_candidate_count - candidates.len()); + debug!("untested_candidates: {}", candidates.len()); + + (candidates, target_candidates) + } + + /// Given that we are performing `test` against `test_place`, this job + /// sorts out what the status of `candidate` will be after the test. See + /// `test_candidates` for the usage of this function. The candidate may + /// be modified to update its `match_pairs`. + /// + /// So, for example, if this candidate is `x @ Some(P0)` and the `Test` is + /// a variant test, then we would modify the candidate to be `(x as + /// Option).0 @ P0` and return the index corresponding to the variant + /// `Some`. + /// + /// However, in some cases, the test may just not be relevant to candidate. + /// For example, suppose we are testing whether `foo.x == 22`, but in one + /// match arm we have `Foo { x: _, ... }`... in that case, the test for + /// the value of `x` has no particular relevance to this candidate. In + /// such cases, this function just returns None without doing anything. + /// This is used by the overall `match_candidates` algorithm to structure + /// the match as a whole. See `match_candidates` for more details. + /// + /// FIXME(#29623). In some cases, we have some tricky choices to make. for + /// example, if we are testing that `x == 22`, but the candidate is `x @ + /// 13..55`, what should we do? In the event that the test is true, we know + /// that the candidate applies, but in the event of false, we don't know + /// that it *doesn't* apply. For now, we return false, indicate that the + /// test does not apply to this candidate, but it might be we can get + /// tighter match code if we do something a bit different. + fn sort_candidate( + &mut self, + test_place: Place<'tcx>, + test: &Test<'tcx>, + candidate: &mut Candidate<'tcx>, + sorted_candidates: &FxIndexMap, Vec<&mut Candidate<'tcx>>>, + ) -> Option> { + // Find the match_pair for this place (if any). At present, + // afaik, there can be at most one. (In the future, if we + // adopted a more general `@` operator, there might be more + // than one, but it'd be very unusual to have two sides that + // both require tests; you'd expect one side to be simplified + // away.) + let (match_pair_index, match_pair) = candidate + .match_pairs + .iter() + .enumerate() + .find(|&(_, mp)| mp.place == Some(test_place))?; + + // If true, the match pair is completely entailed by its corresponding test + // branch, so it can be removed. If false, the match pair is _compatible_ + // with its test branch, but still needs a more specific test. + let fully_matched; + let ret = match (&test.kind, &match_pair.test_case) { + // If we are performing a variant switch, then this + // informs variant patterns, but nothing else. + ( + &TestKind::Switch { adt_def: tested_adt_def }, + &TestCase::Variant { adt_def, variant_index }, + ) => { + assert_eq!(adt_def, tested_adt_def); + fully_matched = true; + Some(TestBranch::Variant(variant_index)) + } + + // If we are performing a switch over integers, then this informs integer + // equality, but nothing else. + // + // FIXME(#29623) we could use PatKind::Range to rule + // things out here, in some cases. + (TestKind::SwitchInt, &TestCase::Constant { value }) + if is_switch_ty(match_pair.pattern_ty) => + { + // An important invariant of candidate sorting is that a candidate + // must not match in multiple branches. For `SwitchInt` tests, adding + // a new value might invalidate that property for range patterns that + // have already been sorted into the failure arm, so we must take care + // not to add such values here. + let is_covering_range = |test_case: &TestCase<'tcx>| { + test_case.as_range().is_some_and(|range| { + matches!(range.contains(value, self.tcx), None | Some(true)) + }) + }; + let is_conflicting_candidate = |candidate: &&mut Candidate<'tcx>| { + candidate + .match_pairs + .iter() + .any(|mp| mp.place == Some(test_place) && is_covering_range(&mp.test_case)) + }; + if sorted_candidates + .get(&TestBranch::Failure) + .is_some_and(|candidates| candidates.iter().any(is_conflicting_candidate)) + { + fully_matched = false; + None + } else { + fully_matched = true; + Some(TestBranch::Constant(value)) + } + } + (TestKind::SwitchInt, TestCase::Range(range)) => { + // When performing a `SwitchInt` test, a range pattern can be + // sorted into the failure arm if it doesn't contain _any_ of + // the values being tested. (This restricts what values can be + // added to the test by subsequent candidates.) + fully_matched = false; + let not_contained = sorted_candidates + .keys() + .filter_map(|br| br.as_constant()) + .all(|val| matches!(range.contains(val, self.tcx), Some(false))); + + not_contained.then(|| { + // No switch values are contained in the pattern range, + // so the pattern can be matched only if this test fails. + TestBranch::Failure + }) + } + + (TestKind::If, TestCase::Constant { value }) => { + fully_matched = true; + let value = value.try_to_bool().unwrap_or_else(|| { + span_bug!(test.span, "expected boolean value but got {value:?}") + }); + Some(if value { TestBranch::Success } else { TestBranch::Failure }) + } + + ( + &TestKind::Len { len: test_len, op: BinOp::Eq }, + &TestCase::Slice { len, variable_length }, + ) => { + match (test_len.cmp(&(len as u64)), variable_length) { + (Ordering::Equal, false) => { + // on true, min_len = len = $actual_length, + // on false, len != $actual_length + fully_matched = true; + Some(TestBranch::Success) + } + (Ordering::Less, _) => { + // test_len < pat_len. If $actual_len = test_len, + // then $actual_len < pat_len and we don't have + // enough elements. + fully_matched = false; + Some(TestBranch::Failure) + } + (Ordering::Equal | Ordering::Greater, true) => { + // This can match both if $actual_len = test_len >= pat_len, + // and if $actual_len > test_len. We can't advance. + fully_matched = false; + None + } + (Ordering::Greater, false) => { + // test_len != pat_len, so if $actual_len = test_len, then + // $actual_len != pat_len. + fully_matched = false; + Some(TestBranch::Failure) + } + } + } + ( + &TestKind::Len { len: test_len, op: BinOp::Ge }, + &TestCase::Slice { len, variable_length }, + ) => { + // the test is `$actual_len >= test_len` + match (test_len.cmp(&(len as u64)), variable_length) { + (Ordering::Equal, true) => { + // $actual_len >= test_len = pat_len, + // so we can match. + fully_matched = true; + Some(TestBranch::Success) + } + (Ordering::Less, _) | (Ordering::Equal, false) => { + // test_len <= pat_len. If $actual_len < test_len, + // then it is also < pat_len, so the test passing is + // necessary (but insufficient). + fully_matched = false; + Some(TestBranch::Success) + } + (Ordering::Greater, false) => { + // test_len > pat_len. If $actual_len >= test_len > pat_len, + // then we know we won't have a match. + fully_matched = false; + Some(TestBranch::Failure) + } + (Ordering::Greater, true) => { + // test_len < pat_len, and is therefore less + // strict. This can still go both ways. + fully_matched = false; + None + } + } + } + + (TestKind::Range(test), TestCase::Range(pat)) => { + if test == pat { + fully_matched = true; + Some(TestBranch::Success) + } else { + fully_matched = false; + // If the testing range does not overlap with pattern range, + // the pattern can be matched only if this test fails. + if !test.overlaps(pat, self.tcx)? { Some(TestBranch::Failure) } else { None } + } + } + (TestKind::Range(range), &TestCase::Constant { value }) => { + fully_matched = false; + if !range.contains(value, self.tcx)? { + // `value` is not contained in the testing range, + // so `value` can be matched only if this test fails. + Some(TestBranch::Failure) + } else { + None + } + } + + (TestKind::Eq { value: test_val, .. }, TestCase::Constant { value: case_val }) => { + if test_val == case_val { + fully_matched = true; + Some(TestBranch::Success) + } else { + fully_matched = false; + Some(TestBranch::Failure) + } + } + + (TestKind::Deref { temp: test_temp, .. }, TestCase::Deref { temp, .. }) + if test_temp == temp => + { + fully_matched = true; + Some(TestBranch::Success) + } + + (TestKind::Never, _) => { + fully_matched = true; + Some(TestBranch::Success) + } + + ( + TestKind::Switch { .. } + | TestKind::SwitchInt { .. } + | TestKind::If + | TestKind::Len { .. } + | TestKind::Range { .. } + | TestKind::Eq { .. } + | TestKind::Deref { .. }, + _, + ) => { + fully_matched = false; + None + } + }; + + if fully_matched { + // Replace the match pair by its sub-pairs. + let match_pair = candidate.match_pairs.remove(match_pair_index); + candidate.match_pairs.extend(match_pair.subpairs); + // Move or-patterns to the end. + candidate.sort_match_pairs(); + } + + ret + } +} diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index 433c3e5aaaeec..aa223435720cf 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -34,6 +34,7 @@ use crate::builder::{ }; // helper functions, broken out by category: +mod buckets; mod match_pair; mod test; mod user_ty; @@ -2172,86 +2173,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { (match_place, test) } - /// Given a test, we partition the input candidates into several buckets. - /// If a candidate matches in exactly one of the branches of `test` - /// (and no other branches), we put it into the corresponding bucket. - /// If it could match in more than one of the branches of `test`, the test - /// doesn't usefully apply to it, and we stop partitioning candidates. - /// - /// Importantly, we also **mutate** the branched candidates to remove match pairs - /// that are entailed by the outcome of the test, and add any sub-pairs of the - /// removed pairs. - /// - /// This returns a pair of - /// - the candidates that weren't sorted; - /// - for each possible outcome of the test, the candidates that match in that outcome. - /// - /// For example: - /// ``` - /// # let (x, y, z) = (true, true, true); - /// match (x, y, z) { - /// (true , _ , true ) => true, // (0) - /// (false, false, _ ) => false, // (1) - /// (_ , true , _ ) => true, // (2) - /// (true , _ , false) => false, // (3) - /// } - /// # ; - /// ``` - /// - /// Assume we are testing on `x`. Conceptually, there are 2 overlapping candidate sets: - /// - If the outcome is that `x` is true, candidates {0, 2, 3} are possible - /// - If the outcome is that `x` is false, candidates {1, 2} are possible - /// - /// Following our algorithm: - /// - Candidate 0 is sorted into outcome `x == true` - /// - Candidate 1 is sorted into outcome `x == false` - /// - Candidate 2 remains unsorted, because testing `x` has no effect on it - /// - Candidate 3 remains unsorted, because a previous candidate (2) was unsorted - /// - This helps preserve the illusion that candidates are tested "in order" - /// - /// The sorted candidates are mutated to remove entailed match pairs: - /// - candidate 0 becomes `[z @ true]` since we know that `x` was `true`; - /// - candidate 1 becomes `[y @ false]` since we know that `x` was `false`. - fn sort_candidates<'b, 'c>( - &mut self, - match_place: Place<'tcx>, - test: &Test<'tcx>, - mut candidates: &'b mut [&'c mut Candidate<'tcx>], - ) -> ( - &'b mut [&'c mut Candidate<'tcx>], - FxIndexMap, Vec<&'b mut Candidate<'tcx>>>, - ) { - // For each of the possible outcomes, collect vector of candidates that apply if the test - // has that particular outcome. - let mut target_candidates: FxIndexMap<_, Vec<&mut Candidate<'_>>> = Default::default(); - - let total_candidate_count = candidates.len(); - - // Sort the candidates into the appropriate vector in `target_candidates`. Note that at some - // point we may encounter a candidate where the test is not relevant; at that point, we stop - // sorting. - while let Some(candidate) = candidates.first_mut() { - let Some(branch) = - self.sort_candidate(match_place, test, candidate, &target_candidates) - else { - break; - }; - let (candidate, rest) = candidates.split_first_mut().unwrap(); - target_candidates.entry(branch).or_insert_with(Vec::new).push(candidate); - candidates = rest; - } - - // At least the first candidate ought to be tested - assert!( - total_candidate_count > candidates.len(), - "{total_candidate_count}, {candidates:#?}" - ); - debug!("tested_candidates: {}", total_candidate_count - candidates.len()); - debug!("untested_candidates: {}", candidates.len()); - - (candidates, target_candidates) - } - /// This is the most subtle part of the match lowering algorithm. At this point, there are /// no fully-satisfied candidates, and no or-patterns to expand, so we actually need to /// perform some sort of test to make progress. diff --git a/compiler/rustc_mir_build/src/builder/matches/test.rs b/compiler/rustc_mir_build/src/builder/matches/test.rs index 1b6d96e49f0c1..46a76a7b9a30b 100644 --- a/compiler/rustc_mir_build/src/builder/matches/test.rs +++ b/compiler/rustc_mir_build/src/builder/matches/test.rs @@ -5,7 +5,6 @@ // identify what tests are needed, perform the tests, and then filter // the candidates based on the result. -use std::cmp::Ordering; use std::sync::Arc; use rustc_data_structures::fx::FxIndexMap; @@ -20,7 +19,7 @@ use rustc_span::{DUMMY_SP, Span, Symbol, sym}; use tracing::{debug, instrument}; use crate::builder::Builder; -use crate::builder::matches::{Candidate, MatchPairTree, Test, TestBranch, TestCase, TestKind}; +use crate::builder::matches::{MatchPairTree, Test, TestBranch, TestCase, TestKind}; impl<'a, 'tcx> Builder<'a, 'tcx> { /// Identifies what test is needed to decide if `match_pair` is applicable. @@ -486,266 +485,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { TerminatorKind::if_(Operand::Move(eq_result), success_block, fail_block), ); } - - /// Given that we are performing `test` against `test_place`, this job - /// sorts out what the status of `candidate` will be after the test. See - /// `test_candidates` for the usage of this function. The candidate may - /// be modified to update its `match_pairs`. - /// - /// So, for example, if this candidate is `x @ Some(P0)` and the `Test` is - /// a variant test, then we would modify the candidate to be `(x as - /// Option).0 @ P0` and return the index corresponding to the variant - /// `Some`. - /// - /// However, in some cases, the test may just not be relevant to candidate. - /// For example, suppose we are testing whether `foo.x == 22`, but in one - /// match arm we have `Foo { x: _, ... }`... in that case, the test for - /// the value of `x` has no particular relevance to this candidate. In - /// such cases, this function just returns None without doing anything. - /// This is used by the overall `match_candidates` algorithm to structure - /// the match as a whole. See `match_candidates` for more details. - /// - /// FIXME(#29623). In some cases, we have some tricky choices to make. for - /// example, if we are testing that `x == 22`, but the candidate is `x @ - /// 13..55`, what should we do? In the event that the test is true, we know - /// that the candidate applies, but in the event of false, we don't know - /// that it *doesn't* apply. For now, we return false, indicate that the - /// test does not apply to this candidate, but it might be we can get - /// tighter match code if we do something a bit different. - pub(super) fn sort_candidate( - &mut self, - test_place: Place<'tcx>, - test: &Test<'tcx>, - candidate: &mut Candidate<'tcx>, - sorted_candidates: &FxIndexMap, Vec<&mut Candidate<'tcx>>>, - ) -> Option> { - // Find the match_pair for this place (if any). At present, - // afaik, there can be at most one. (In the future, if we - // adopted a more general `@` operator, there might be more - // than one, but it'd be very unusual to have two sides that - // both require tests; you'd expect one side to be simplified - // away.) - let (match_pair_index, match_pair) = candidate - .match_pairs - .iter() - .enumerate() - .find(|&(_, mp)| mp.place == Some(test_place))?; - - // If true, the match pair is completely entailed by its corresponding test - // branch, so it can be removed. If false, the match pair is _compatible_ - // with its test branch, but still needs a more specific test. - let fully_matched; - let ret = match (&test.kind, &match_pair.test_case) { - // If we are performing a variant switch, then this - // informs variant patterns, but nothing else. - ( - &TestKind::Switch { adt_def: tested_adt_def }, - &TestCase::Variant { adt_def, variant_index }, - ) => { - assert_eq!(adt_def, tested_adt_def); - fully_matched = true; - Some(TestBranch::Variant(variant_index)) - } - - // If we are performing a switch over integers, then this informs integer - // equality, but nothing else. - // - // FIXME(#29623) we could use PatKind::Range to rule - // things out here, in some cases. - (TestKind::SwitchInt, &TestCase::Constant { value }) - if is_switch_ty(match_pair.pattern_ty) => - { - // An important invariant of candidate sorting is that a candidate - // must not match in multiple branches. For `SwitchInt` tests, adding - // a new value might invalidate that property for range patterns that - // have already been sorted into the failure arm, so we must take care - // not to add such values here. - let is_covering_range = |test_case: &TestCase<'tcx>| { - test_case.as_range().is_some_and(|range| { - matches!(range.contains(value, self.tcx), None | Some(true)) - }) - }; - let is_conflicting_candidate = |candidate: &&mut Candidate<'tcx>| { - candidate - .match_pairs - .iter() - .any(|mp| mp.place == Some(test_place) && is_covering_range(&mp.test_case)) - }; - if sorted_candidates - .get(&TestBranch::Failure) - .is_some_and(|candidates| candidates.iter().any(is_conflicting_candidate)) - { - fully_matched = false; - None - } else { - fully_matched = true; - Some(TestBranch::Constant(value)) - } - } - (TestKind::SwitchInt, TestCase::Range(range)) => { - // When performing a `SwitchInt` test, a range pattern can be - // sorted into the failure arm if it doesn't contain _any_ of - // the values being tested. (This restricts what values can be - // added to the test by subsequent candidates.) - fully_matched = false; - let not_contained = sorted_candidates - .keys() - .filter_map(|br| br.as_constant()) - .all(|val| matches!(range.contains(val, self.tcx), Some(false))); - - not_contained.then(|| { - // No switch values are contained in the pattern range, - // so the pattern can be matched only if this test fails. - TestBranch::Failure - }) - } - - (TestKind::If, TestCase::Constant { value }) => { - fully_matched = true; - let value = value.try_to_bool().unwrap_or_else(|| { - span_bug!(test.span, "expected boolean value but got {value:?}") - }); - Some(if value { TestBranch::Success } else { TestBranch::Failure }) - } - - ( - &TestKind::Len { len: test_len, op: BinOp::Eq }, - &TestCase::Slice { len, variable_length }, - ) => { - match (test_len.cmp(&(len as u64)), variable_length) { - (Ordering::Equal, false) => { - // on true, min_len = len = $actual_length, - // on false, len != $actual_length - fully_matched = true; - Some(TestBranch::Success) - } - (Ordering::Less, _) => { - // test_len < pat_len. If $actual_len = test_len, - // then $actual_len < pat_len and we don't have - // enough elements. - fully_matched = false; - Some(TestBranch::Failure) - } - (Ordering::Equal | Ordering::Greater, true) => { - // This can match both if $actual_len = test_len >= pat_len, - // and if $actual_len > test_len. We can't advance. - fully_matched = false; - None - } - (Ordering::Greater, false) => { - // test_len != pat_len, so if $actual_len = test_len, then - // $actual_len != pat_len. - fully_matched = false; - Some(TestBranch::Failure) - } - } - } - ( - &TestKind::Len { len: test_len, op: BinOp::Ge }, - &TestCase::Slice { len, variable_length }, - ) => { - // the test is `$actual_len >= test_len` - match (test_len.cmp(&(len as u64)), variable_length) { - (Ordering::Equal, true) => { - // $actual_len >= test_len = pat_len, - // so we can match. - fully_matched = true; - Some(TestBranch::Success) - } - (Ordering::Less, _) | (Ordering::Equal, false) => { - // test_len <= pat_len. If $actual_len < test_len, - // then it is also < pat_len, so the test passing is - // necessary (but insufficient). - fully_matched = false; - Some(TestBranch::Success) - } - (Ordering::Greater, false) => { - // test_len > pat_len. If $actual_len >= test_len > pat_len, - // then we know we won't have a match. - fully_matched = false; - Some(TestBranch::Failure) - } - (Ordering::Greater, true) => { - // test_len < pat_len, and is therefore less - // strict. This can still go both ways. - fully_matched = false; - None - } - } - } - - (TestKind::Range(test), TestCase::Range(pat)) => { - if test == pat { - fully_matched = true; - Some(TestBranch::Success) - } else { - fully_matched = false; - // If the testing range does not overlap with pattern range, - // the pattern can be matched only if this test fails. - if !test.overlaps(pat, self.tcx)? { Some(TestBranch::Failure) } else { None } - } - } - (TestKind::Range(range), &TestCase::Constant { value }) => { - fully_matched = false; - if !range.contains(value, self.tcx)? { - // `value` is not contained in the testing range, - // so `value` can be matched only if this test fails. - Some(TestBranch::Failure) - } else { - None - } - } - - (TestKind::Eq { value: test_val, .. }, TestCase::Constant { value: case_val }) => { - if test_val == case_val { - fully_matched = true; - Some(TestBranch::Success) - } else { - fully_matched = false; - Some(TestBranch::Failure) - } - } - - (TestKind::Deref { temp: test_temp, .. }, TestCase::Deref { temp, .. }) - if test_temp == temp => - { - fully_matched = true; - Some(TestBranch::Success) - } - - (TestKind::Never, _) => { - fully_matched = true; - Some(TestBranch::Success) - } - - ( - TestKind::Switch { .. } - | TestKind::SwitchInt { .. } - | TestKind::If - | TestKind::Len { .. } - | TestKind::Range { .. } - | TestKind::Eq { .. } - | TestKind::Deref { .. }, - _, - ) => { - fully_matched = false; - None - } - }; - - if fully_matched { - // Replace the match pair by its sub-pairs. - let match_pair = candidate.match_pairs.remove(match_pair_index); - candidate.match_pairs.extend(match_pair.subpairs); - // Move or-patterns to the end. - candidate.sort_match_pairs(); - } - - ret - } } -fn is_switch_ty(ty: Ty<'_>) -> bool { +/// Returns true if this type be used with [`TestKind::SwitchInt`]. +pub(crate) fn is_switch_ty(ty: Ty<'_>) -> bool { ty.is_integral() || ty.is_char() } From bc11e7e142641c5784dcbf664a043adcc83c4a5e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 13 Dec 2025 20:28:05 +1100 Subject: [PATCH 3/8] Rename match-candidate partitioning methods --- .../src/builder/matches/buckets.rs | 56 ++++++++++--------- .../src/builder/matches/mod.rs | 9 +-- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_mir_build/src/builder/matches/buckets.rs b/compiler/rustc_mir_build/src/builder/matches/buckets.rs index 0643704ed35ad..f53e73d406c3c 100644 --- a/compiler/rustc_mir_build/src/builder/matches/buckets.rs +++ b/compiler/rustc_mir_build/src/builder/matches/buckets.rs @@ -9,6 +9,14 @@ use crate::builder::Builder; use crate::builder::matches::test::is_switch_ty; use crate::builder::matches::{Candidate, Test, TestBranch, TestCase, TestKind}; +/// Output of [`Builder::partition_candidates_into_buckets`]. +pub(crate) struct PartitionedCandidates<'tcx, 'b, 'c> { + /// For each possible outcome of the test, the candidates that are matched in that outcome. + pub(crate) target_candidates: FxIndexMap, Vec<&'b mut Candidate<'tcx>>>, + /// The remaining candidates that weren't associated with any test outcome. + pub(crate) remaining_candidates: &'b mut [&'c mut Candidate<'tcx>], +} + impl<'a, 'tcx> Builder<'a, 'tcx> { /// Given a test, we partition the input candidates into several buckets. /// If a candidate matches in exactly one of the branches of `test` @@ -20,10 +28,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// that are entailed by the outcome of the test, and add any sub-pairs of the /// removed pairs. /// - /// This returns a pair of - /// - the candidates that weren't sorted; - /// - for each possible outcome of the test, the candidates that match in that outcome. - /// /// For example: /// ``` /// # let (x, y, z) = (true, true, true); @@ -41,36 +45,33 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// - If the outcome is that `x` is false, candidates {1, 2} are possible /// /// Following our algorithm: - /// - Candidate 0 is sorted into outcome `x == true` - /// - Candidate 1 is sorted into outcome `x == false` - /// - Candidate 2 remains unsorted, because testing `x` has no effect on it - /// - Candidate 3 remains unsorted, because a previous candidate (2) was unsorted + /// - Candidate 0 is bucketed into outcome `x == true` + /// - Candidate 1 is bucketed into outcome `x == false` + /// - Candidate 2 remains unbucketed, because testing `x` has no effect on it + /// - Candidate 3 remains unbucketed, because a previous candidate (2) was unbucketed /// - This helps preserve the illusion that candidates are tested "in order" /// - /// The sorted candidates are mutated to remove entailed match pairs: + /// The bucketed candidates are mutated to remove entailed match pairs: /// - candidate 0 becomes `[z @ true]` since we know that `x` was `true`; /// - candidate 1 becomes `[y @ false]` since we know that `x` was `false`. - pub(super) fn sort_candidates<'b, 'c>( + pub(super) fn partition_candidates_into_buckets<'b, 'c>( &mut self, match_place: Place<'tcx>, test: &Test<'tcx>, mut candidates: &'b mut [&'c mut Candidate<'tcx>], - ) -> ( - &'b mut [&'c mut Candidate<'tcx>], - FxIndexMap, Vec<&'b mut Candidate<'tcx>>>, - ) { - // For each of the possible outcomes, collect vector of candidates that apply if the test + ) -> PartitionedCandidates<'tcx, 'b, 'c> { + // For each of the possible outcomes, collect a vector of candidates that apply if the test // has that particular outcome. let mut target_candidates: FxIndexMap<_, Vec<&mut Candidate<'_>>> = Default::default(); let total_candidate_count = candidates.len(); - // Sort the candidates into the appropriate vector in `target_candidates`. Note that at some - // point we may encounter a candidate where the test is not relevant; at that point, we stop - // sorting. + // Partition the candidates into the appropriate vector in `target_candidates`. + // Note that at some point we may encounter a candidate where the test is not relevant; + // at that point, we stop partitioning. while let Some(candidate) = candidates.first_mut() { let Some(branch) = - self.sort_candidate(match_place, test, candidate, &target_candidates) + self.choose_bucket_for_candidate(match_place, test, candidate, &target_candidates) else { break; }; @@ -87,7 +88,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { debug!("tested_candidates: {}", total_candidate_count - candidates.len()); debug!("untested_candidates: {}", candidates.len()); - (candidates, target_candidates) + PartitionedCandidates { target_candidates, remaining_candidates: candidates } } /// Given that we are performing `test` against `test_place`, this job @@ -115,12 +116,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// that it *doesn't* apply. For now, we return false, indicate that the /// test does not apply to this candidate, but it might be we can get /// tighter match code if we do something a bit different. - fn sort_candidate( + fn choose_bucket_for_candidate( &mut self, test_place: Place<'tcx>, test: &Test<'tcx>, candidate: &mut Candidate<'tcx>, - sorted_candidates: &FxIndexMap, Vec<&mut Candidate<'tcx>>>, + // Other candidates that have already been partitioned into a bucket for this test, if any + prior_candidates: &FxIndexMap, Vec<&mut Candidate<'tcx>>>, ) -> Option> { // Find the match_pair for this place (if any). At present, // afaik, there can be at most one. (In the future, if we @@ -155,13 +157,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // // FIXME(#29623) we could use PatKind::Range to rule // things out here, in some cases. + // + // FIXME(Zalathar): Is the `is_switch_ty` test unnecessary? (TestKind::SwitchInt, &TestCase::Constant { value }) if is_switch_ty(match_pair.pattern_ty) => { - // An important invariant of candidate sorting is that a candidate + // An important invariant of candidate bucketing is that a candidate // must not match in multiple branches. For `SwitchInt` tests, adding // a new value might invalidate that property for range patterns that - // have already been sorted into the failure arm, so we must take care + // have already been partitioned into the failure arm, so we must take care // not to add such values here. let is_covering_range = |test_case: &TestCase<'tcx>| { test_case.as_range().is_some_and(|range| { @@ -174,7 +178,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .iter() .any(|mp| mp.place == Some(test_place) && is_covering_range(&mp.test_case)) }; - if sorted_candidates + if prior_candidates .get(&TestBranch::Failure) .is_some_and(|candidates| candidates.iter().any(is_conflicting_candidate)) { @@ -191,7 +195,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // the values being tested. (This restricts what values can be // added to the test by subsequent candidates.) fully_matched = false; - let not_contained = sorted_candidates + let not_contained = prior_candidates .keys() .filter_map(|br| br.as_constant()) .all(|val| matches!(range.contains(val, self.tcx), Some(false))); diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index aa223435720cf..5074ce3367986 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -27,6 +27,7 @@ use tracing::{debug, instrument}; use crate::builder::ForGuard::{self, OutsideGuard, RefWithinGuard}; use crate::builder::expr::as_place::PlaceBuilder; +use crate::builder::matches::buckets::PartitionedCandidates; use crate::builder::matches::user_ty::ProjectedUserTypesNode; use crate::builder::scope::DropKind; use crate::builder::{ @@ -1069,7 +1070,7 @@ struct Candidate<'tcx> { /// Key mutations include: /// /// - When a match pair is fully satisfied by a test, it is removed from the - /// list, and its subpairs are added instead (see [`Builder::sort_candidate`]). + /// list, and its subpairs are added instead (see [`Builder::choose_bucket_for_candidate`]). /// - During or-pattern expansion, any leading or-pattern is removed, and is /// converted into subcandidates (see [`Builder::expand_and_match_or_candidates`]). /// - After a candidate's subcandidates have been lowered, a copy of any remaining @@ -1254,7 +1255,7 @@ struct Ascription<'tcx> { /// /// Created by [`MatchPairTree::for_pattern`], and then inspected primarily by: /// - [`Builder::pick_test_for_match_pair`] (to choose a test) -/// - [`Builder::sort_candidate`] (to see how the test interacts with a match pair) +/// - [`Builder::choose_bucket_for_candidate`] (to see how the test interacts with a match pair) /// /// Note that or-patterns are not tested directly like the other variants. /// Instead they participate in or-pattern expansion, where they are transformed into @@ -2284,8 +2285,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // For each of the N possible test outcomes, build the vector of candidates that applies if // the test has that particular outcome. This also mutates the candidates to remove match // pairs that are fully satisfied by the relevant outcome. - let (remaining_candidates, target_candidates) = - self.sort_candidates(match_place, &test, candidates); + let PartitionedCandidates { target_candidates, remaining_candidates } = + self.partition_candidates_into_buckets(match_place, &test, candidates); // The block that we should branch to if none of the `target_candidates` match. let remainder_start = self.cfg.start_new_block(); From 5e42f8b22f452fd23f60aab137bcd1af5f4f9a57 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 14 Dec 2025 23:16:05 +1100 Subject: [PATCH 4/8] Move ambient cdb discovery from compiletest to bootstrap --- src/bootstrap/src/core/build_steps/test.rs | 4 ++ src/bootstrap/src/core/debuggers/cdb.rs | 41 +++++++++++++++++++ src/bootstrap/src/core/debuggers/mod.rs | 2 + src/tools/compiletest/src/debuggers.rs | 47 +--------------------- src/tools/compiletest/src/lib.rs | 2 +- 5 files changed, 49 insertions(+), 47 deletions(-) create mode 100644 src/bootstrap/src/core/debuggers/cdb.rs diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 9f8bb4dea54fd..67214d832444c 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2181,6 +2181,10 @@ Please disable assertions with `rust.debug-assertions = false`. } if mode == CompiletestMode::Debuginfo { + if let Some(debuggers::Cdb { cdb }) = debuggers::discover_cdb(target) { + cmd.arg("--cdb").arg(cdb); + } + if let Some(debuggers::Gdb { gdb }) = debuggers::discover_gdb(builder, android.as_ref()) { cmd.arg("--gdb").arg(gdb.as_ref()); diff --git a/src/bootstrap/src/core/debuggers/cdb.rs b/src/bootstrap/src/core/debuggers/cdb.rs new file mode 100644 index 0000000000000..a19b70477ecfe --- /dev/null +++ b/src/bootstrap/src/core/debuggers/cdb.rs @@ -0,0 +1,41 @@ +use std::env; +use std::path::PathBuf; + +use crate::core::config::TargetSelection; + +pub(crate) struct Cdb { + pub(crate) cdb: PathBuf, +} + +/// FIXME: This CDB discovery code was very questionable when it was in +/// compiletest, and it's just as questionable now that it's in bootstrap. +pub(crate) fn discover_cdb(target: TargetSelection) -> Option { + if !cfg!(windows) || !target.ends_with("-pc-windows-msvc") { + return None; + } + + let pf86 = + PathBuf::from(env::var_os("ProgramFiles(x86)").or_else(|| env::var_os("ProgramFiles"))?); + let cdb_arch = if cfg!(target_arch = "x86") { + "x86" + } else if cfg!(target_arch = "x86_64") { + "x64" + } else if cfg!(target_arch = "aarch64") { + "arm64" + } else if cfg!(target_arch = "arm") { + "arm" + } else { + return None; // No compatible CDB.exe in the Windows 10 SDK + }; + + let mut path = pf86; + path.push(r"Windows Kits\10\Debuggers"); // We could check 8.1 etc. too? + path.push(cdb_arch); + path.push(r"cdb.exe"); + + if !path.exists() { + return None; + } + + Some(Cdb { cdb: path }) +} diff --git a/src/bootstrap/src/core/debuggers/mod.rs b/src/bootstrap/src/core/debuggers/mod.rs index a11eda8e686e3..65d76e1141896 100644 --- a/src/bootstrap/src/core/debuggers/mod.rs +++ b/src/bootstrap/src/core/debuggers/mod.rs @@ -1,8 +1,10 @@ //! Code for discovering debuggers and debugger-related configuration, so that //! it can be passed to compiletest when running debuginfo tests. +pub(crate) use self::cdb::{Cdb, discover_cdb}; pub(crate) use self::gdb::{Gdb, discover_gdb}; pub(crate) use self::lldb::{Lldb, discover_lldb}; +mod cdb; mod gdb; mod lldb; diff --git a/src/tools/compiletest/src/debuggers.rs b/src/tools/compiletest/src/debuggers.rs index b0d0372454e2d..4a1a74ddfe1bd 100644 --- a/src/tools/compiletest/src/debuggers.rs +++ b/src/tools/compiletest/src/debuggers.rs @@ -2,7 +2,7 @@ use std::env; use std::process::Command; use std::sync::Arc; -use camino::{Utf8Path, Utf8PathBuf}; +use camino::Utf8Path; use crate::common::{Config, Debugger}; @@ -54,51 +54,6 @@ pub(crate) fn configure_lldb(config: &Config) -> Option> { Some(Arc::new(Config { debugger: Some(Debugger::Lldb), ..config.clone() })) } -/// Returns `true` if the given target is a MSVC target for the purposes of CDB testing. -fn is_pc_windows_msvc_target(target: &str) -> bool { - target.ends_with("-pc-windows-msvc") -} - -/// FIXME: this is very questionable... -fn find_cdb(target: &str) -> Option { - if !(cfg!(windows) && is_pc_windows_msvc_target(target)) { - return None; - } - - let pf86 = Utf8PathBuf::from_path_buf( - env::var_os("ProgramFiles(x86)").or_else(|| env::var_os("ProgramFiles"))?.into(), - ) - .unwrap(); - let cdb_arch = if cfg!(target_arch = "x86") { - "x86" - } else if cfg!(target_arch = "x86_64") { - "x64" - } else if cfg!(target_arch = "aarch64") { - "arm64" - } else if cfg!(target_arch = "arm") { - "arm" - } else { - return None; // No compatible CDB.exe in the Windows 10 SDK - }; - - let mut path = pf86; - path.push(r"Windows Kits\10\Debuggers"); // We could check 8.1 etc. too? - path.push(cdb_arch); - path.push(r"cdb.exe"); - - if !path.exists() { - return None; - } - - Some(path) -} - -/// Returns Path to CDB -pub(crate) fn discover_cdb(cdb: Option, target: &str) -> Option { - let cdb = cdb.map(Utf8PathBuf::from).or_else(|| find_cdb(target)); - cdb -} - pub(crate) fn query_cdb_version(cdb: &Utf8Path) -> Option<[u16; 4]> { let mut version = None; if let Ok(output) = Command::new(cdb).arg("/version").output() { diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 71dad36337932..90b1c10308a27 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -263,7 +263,7 @@ fn parse_config(args: Vec) -> Config { let adb_device_status = target.contains("android") && adb_test_dir.is_some(); // FIXME: `cdb_version` is *derived* from cdb, but it's *not* technically a config! - let cdb = debuggers::discover_cdb(matches.opt_str("cdb"), &target); + let cdb = matches.opt_str("cdb").map(Utf8PathBuf::from); let cdb_version = cdb.as_deref().and_then(debuggers::query_cdb_version); // FIXME: `gdb_version` is *derived* from gdb, but it's *not* technically a config! let gdb = matches.opt_str("gdb").map(Utf8PathBuf::from); From efaa0c067f681e42c39f4594042bafd8d02e3e7a Mon Sep 17 00:00:00 2001 From: Flakebi Date: Sat, 13 Dec 2025 19:16:32 +0100 Subject: [PATCH 5/8] Improve amdgpu docs: Mention device-libs and xnack Mention amdgpu-device-libs for allocator and println support and mention the necessary target flag for GPUs from some some series without xnack support. For the last half year, there were no major changes in the amdgpu-device-libs crate (in fact, there was no need to update the crate), so I believe it is stable enough to mention it here. The xnack-support flag is something I stumbled upon recently, when more complex printlns failed on a certain GPU. --- .../rustc/src/platform-support/amdgcn-amd-amdhsa.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/doc/rustc/src/platform-support/amdgcn-amd-amdhsa.md b/src/doc/rustc/src/platform-support/amdgcn-amd-amdhsa.md index 16152dd2dad51..dbdb96283a5fa 100644 --- a/src/doc/rustc/src/platform-support/amdgcn-amd-amdhsa.md +++ b/src/doc/rustc/src/platform-support/amdgcn-amd-amdhsa.md @@ -20,8 +20,8 @@ The format of binaries is a linked ELF. Binaries must be built with no-std. They can use `core` and `alloc` (`alloc` only if an allocator is supplied). -At least one function needs to use the `"gpu-kernel"` calling convention and should be marked with `no_mangle` for simplicity. -Functions using the `"gpu-kernel"` calling convention are kernel entrypoints and can be used from the host runtime. +At least one function should use the `"gpu-kernel"` calling convention and should be marked with `no_mangle` or `export_name`. +Functions using the `"gpu-kernel"` calling convention are kernel entrypoints and can be launched from the host runtime. ## Building the target @@ -34,6 +34,9 @@ The generations are exposed as different target-cpus in the backend. As there are many, Rust does not ship pre-compiled libraries for this target. Therefore, you have to build your own copy of `core` by using `cargo -Zbuild-std=core` or similar. +An allocator and `println!()` support is provided by the [`amdgpu-device-libs`] crate. +Both features rely on the [HIP] runtime. + To build a binary, create a no-std library: ```rust,ignore (platform-specific) // src/lib.rs @@ -65,6 +68,8 @@ lto = true The target-cpu must be from the list [supported by LLVM] (or printed with `rustc --target amdgcn-amd-amdhsa --print target-cpus`). The GPU version on the current system can be found e.g. with [`rocminfo`]. +For a GPU series that has xnack support but the target GPU has not, the `-xnack-support` target-feature needs to be enabled. +I.e. if the ISA info as printed with [`rocminfo`] says something about `xnack-`, e.g. `gfx1010:xnack-`, add `-Ctarget-feature=-xnack-support` to the rustflags. Example `.cargo/config.toml` file to set the target and GPU generation: ```toml @@ -72,6 +77,7 @@ Example `.cargo/config.toml` file to set the target and GPU generation: [build] target = "amdgcn-amd-amdhsa" rustflags = ["-Ctarget-cpu=gfx1100"] +# Add "-Ctarget-feature=-xnack-support" for xnack- GPUs (see above) [unstable] build-std = ["core"] # Optional: "alloc" @@ -85,8 +91,6 @@ Example code on how to load a compiled binary and run it is available in [ROCm e On Linux, binaries can also run through the HSA runtime as implemented in [ROCR-Runtime]. - -