From b2a0284bc01d7fcd531e0b57bc6a9eb124a1a968 Mon Sep 17 00:00:00 2001 From: daiyongyuan <1533208939@qq.com> Date: Fri, 25 Sep 2026 10:31:56 +0800 Subject: [PATCH] refactor: compare only W0 bank hashes --- src/nodes/bemu/native/ffi/callbacks.rs | 75 ++++++++------------ src/nodes/bemu/native/ffi/mod.rs | 2 +- src/nodes/bemu/src/trace/btrace.rs | 12 +--- src/nodes/lib/bank-hash/src/comparator.rs | 25 ++++--- src/nodes/lib/bank-hash/src/lib.rs | 14 +--- src/nodes/lib/bank-hash/tests/w0.rs | 85 +++++++++++++++++++++++ src/nodes/lib/rtl-trace/src/banktrace.rs | 4 +- src/nodes/lib/rtl-trace/src/dpi.rs | 14 +--- 8 files changed, 137 insertions(+), 94 deletions(-) create mode 100644 src/nodes/lib/bank-hash/tests/w0.rs diff --git a/src/nodes/bemu/native/ffi/callbacks.rs b/src/nodes/bemu/native/ffi/callbacks.rs index 32db67f0..d34fb0cf 100644 --- a/src/nodes/bemu/native/ffi/callbacks.rs +++ b/src/nodes/bemu/native/ffi/callbacks.rs @@ -32,7 +32,8 @@ pub extern "C" fn buckyball_exec(state: *mut c_void, funct7: u8, xs1: u64, xs2: let trace = &mut state.trace as *mut TraceState; let enable = funct7 >> 4; let btrace = state.trace.btrace_enabled() - && matches!(enable, 1..=4) + && pc != 0 + && matches!(enable, 2..=4) && !matches!(funct7 as u32, FUNCT7_MSET | FUNCT7_MVIN_MMIO); if btrace { state.bank_scoreboard.issue(inst_id); @@ -117,64 +118,46 @@ pub extern "C" fn buckyball_exec(state: *mut c_void, funct7: u8, xs1: u64, xs2: if btrace { bank_scoreboard.complete(inst_id); let op_type = format!("funct7_{}", funct7); - let r0_enabled = matches!(enable, 1 | 3 | 4); - let r1_enabled = enable == 4; - let w0_enabled = matches!(enable, 2 | 3 | 4); - let r0_vbank = (xs1 & 0x3ff) as u32; - let r1_vbank = ((xs1 >> 10) & 0x3ff) as u32; let w0_vbank = ((xs1 >> 20) & 0x3ff) as u32; - let mut hashes = std::collections::BTreeMap::new(); - for (enabled, vbank_id) in [(r0_enabled, r0_vbank), (r1_enabled, r1_vbank), (w0_enabled, w0_vbank)] { - if !enabled { - continue; - } - if hashes.contains_key(&vbank_id) { - continue; - } - let mut status_hash = 0; - let cols = if crate::config::is_shared_vbank(vbank_id as u64) { + let mut status_hash = 0; + let cols = if crate::config::is_shared_vbank(w0_vbank as u64) { + let shared = shared_memory + .as_ref() + .expect("shared bank storage is unavailable") + .banks_mut(); + let core = *hart_id % (shared.cfgs.len() / shared.virtual_bank_count); + shared.cfgs[core * shared.virtual_bank_count + w0_vbank as usize].cols + } else { + bank_cfgs[w0_vbank as usize].cols + }; + for group_id in 0..cols as u32 { + let physical_hash = if crate::config::is_shared_vbank(w0_vbank as u64) { let shared = shared_memory .as_ref() .expect("shared bank storage is unavailable") .banks_mut(); - let core = *hart_id % (shared.cfgs.len() / shared.virtual_bank_count); - shared.cfgs[core * shared.virtual_bank_count + vbank_id as usize].cols + let pbank_id = shared + .map + .resolve_hart_group(*hart_id, w0_vbank, group_id) + .unwrap_or_else(|| panic!("unmapped shared vbank {w0_vbank} group {group_id}")); + shared.storage[pbank_id].status_hash() } else { - bank_cfgs[vbank_id as usize].cols + let pbank_id = bank_map + .resolve_group(w0_vbank, group_id) + .unwrap_or_else(|| panic!("unmapped vbank {w0_vbank} group {group_id}")); + banks[pbank_id].status_hash() }; - for group_id in 0..cols as u32 { - let physical_hash = if crate::config::is_shared_vbank(vbank_id as u64) { - let shared = shared_memory - .as_ref() - .expect("shared bank storage is unavailable") - .banks_mut(); - let pbank_id = shared - .map - .resolve_hart_group(*hart_id, vbank_id, group_id) - .unwrap_or_else(|| panic!("unmapped shared vbank {vbank_id} group {group_id}")); - shared.storage[pbank_id].status_hash() - } else { - let pbank_id = bank_map - .resolve_group(vbank_id, group_id) - .unwrap_or_else(|| panic!("unmapped vbank {vbank_id} group {group_id}")); - banks[pbank_id].status_hash() - }; - status_hash = combine_bank_hash(status_hash, group_id, physical_hash); - } - hashes.insert(vbank_id, status_hash); + status_hash = combine_bank_hash(status_hash, group_id, physical_hash); } - let slot = |enabled: bool, vbank_id: u32| BTraceBank { - vbank_id: if enabled { vbank_id } else { INVALID_VBANK }, - hash: if enabled { hashes[&vbank_id] } else { 0 }, - }; unsafe { with_trace_ptr(trace, || { crate::trace::bemu_btrace( inst_id, *hart_id as u64, - slot(r0_enabled, r0_vbank), - slot(r1_enabled, r1_vbank), - slot(w0_enabled, w0_vbank), + BTraceBank { + vbank_id: w0_vbank, + hash: status_hash, + }, funct7 as u32, &op_type, pc, diff --git a/src/nodes/bemu/native/ffi/mod.rs b/src/nodes/bemu/native/ffi/mod.rs index 6f1ac8d5..587f4a73 100644 --- a/src/nodes/bemu/native/ffi/mod.rs +++ b/src/nodes/bemu/native/ffi/mod.rs @@ -1,4 +1,4 @@ -use bebop_bank_hash::{combine_bank_hash, BTraceBank, INVALID_VBANK}; +use bebop_bank_hash::{combine_bank_hash, BTraceBank}; use bebop_bemu_profile::{BemuProfile, BemuProfileReport}; use bebop_clint::Clint; use bebop_dtb::DtbBuilder; diff --git a/src/nodes/bemu/src/trace/btrace.rs b/src/nodes/bemu/src/trace/btrace.rs index a96291a7..18ff039e 100644 --- a/src/nodes/bemu/src/trace/btrace.rs +++ b/src/nodes/bemu/src/trace/btrace.rs @@ -40,12 +40,9 @@ pub(super) fn init(log_dir: &Path, enabled: bool) -> io::Result { }) } -#[allow(clippy::too_many_arguments)] pub fn bemu_btrace( inst_id: u64, hart_id: u64, - r0: BTraceBank, - r1: BTraceBank, w0: BTraceBank, funct7: u32, op_type: &str, @@ -57,8 +54,6 @@ pub fn bemu_btrace( BTraceSource::Bemu, inst_id, hart_id, - r0, - r1, w0, funct7, op_type, @@ -67,10 +62,9 @@ pub fn bemu_btrace( Some(format!("{GOLDEN_RECORD_FILE}:{line_number}")), ); - if let (Some(file), Ok(line)) = (trace.btrace.golden_record_file.as_mut(), record.to_ndjson()) { - file.write_all(line.as_bytes()).ok(); - file.flush().ok(); - } + let file = trace.btrace.golden_record_file.as_mut().expect("BTrace is enabled"); + file.write_all(record.to_ndjson().unwrap().as_bytes()).unwrap(); + file.flush().unwrap(); observe(&record); }); } diff --git a/src/nodes/lib/bank-hash/src/comparator.rs b/src/nodes/lib/bank-hash/src/comparator.rs index fa5871f2..f8bd8f21 100644 --- a/src/nodes/lib/bank-hash/src/comparator.rs +++ b/src/nodes/lib/bank-hash/src/comparator.rs @@ -33,8 +33,8 @@ pub struct Comparison { pub result: CompareResult, pub hart_id: u64, pub inst_id: u64, - pub subject: Option<[BTraceBank; 3]>, - pub golden: Option<[BTraceBank; 3]>, + pub subject: Option, + pub golden: Option, #[serde(skip_serializing_if = "Option::is_none")] pub funct7: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -43,7 +43,7 @@ pub struct Comparison { pub(crate) fn compare(key: CompareKey, rtl: Option<&BTraceRecord>, bemu: Option<&BTraceRecord>) -> Comparison { let result = match (rtl, bemu) { - (Some(rtl), Some(bemu)) if [rtl.r0, rtl.r1, rtl.w0] == [bemu.r0, bemu.r1, bemu.w0] => CompareResult::Pass, + (Some(rtl), Some(bemu)) if rtl.w0 == bemu.w0 => CompareResult::Pass, (Some(_), Some(_)) => CompareResult::Mismatch, (None, Some(_)) => CompareResult::MissingSubject, (Some(_), None) => CompareResult::MissingGolden, @@ -55,8 +55,8 @@ pub(crate) fn compare(key: CompareKey, rtl: Option<&BTraceRecord>, bemu: Option< result, hart_id: key.hart_id, inst_id: key.inst_id, - subject: rtl.map(|record| [record.r0, record.r1, record.w0]), - golden: bemu.map(|record| [record.r0, record.r1, record.w0]), + subject: rtl.map(|record| record.w0), + golden: bemu.map(|record| record.w0), funct7: bemu .map(|record| record.funct7) .or_else(|| rtl.map(|record| record.funct7)), @@ -67,10 +67,17 @@ pub(crate) fn compare(key: CompareKey, rtl: Option<&BTraceRecord>, bemu: Option< } fn records_by_key(records: impl IntoIterator) -> BTreeMap { - records - .into_iter() - .map(|record| (CompareKey::from(&record), record)) - .collect() + let mut keyed = BTreeMap::new(); + for record in records { + let key = CompareKey::from(&record); + assert!( + keyed.insert(key, record).is_none(), + "duplicate BTrace record: hart={} inst={}", + key.hart_id, + key.inst_id + ); + } + keyed } pub fn compare_offline( diff --git a/src/nodes/lib/bank-hash/src/lib.rs b/src/nodes/lib/bank-hash/src/lib.rs index bd2e1bdb..09976e2d 100644 --- a/src/nodes/lib/bank-hash/src/lib.rs +++ b/src/nodes/lib/bank-hash/src/lib.rs @@ -17,8 +17,6 @@ pub enum BTraceSource { Bemu, } -pub const INVALID_VBANK: u32 = u32::MAX; - #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct BTraceBank { pub vbank_id: u32, @@ -32,19 +30,14 @@ pub enum BTraceTime { VerilatorTime(u64), } -/// comparison record: . -/// -/// The remaining fields are diagnostic metadata and never participate in -/// record alignment. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(deny_unknown_fields)] pub struct BTraceRecord { #[serde(rename = "type")] pub record_type: String, pub source: BTraceSource, pub inst_id: u64, pub hart_id: u64, - pub r0: BTraceBank, - pub r1: BTraceBank, pub w0: BTraceBank, pub funct7: u32, pub op_type: String, @@ -59,13 +52,10 @@ pub struct BTraceRecord { } impl BTraceRecord { - #[allow(clippy::too_many_arguments)] pub fn new( source: BTraceSource, inst_id: u64, hart_id: u64, - r0: BTraceBank, - r1: BTraceBank, w0: BTraceBank, funct7: u32, op_type: impl Into, @@ -83,8 +73,6 @@ impl BTraceRecord { source, inst_id, hart_id, - r0, - r1, w0, funct7, op_type: op_type.into(), diff --git a/src/nodes/lib/bank-hash/tests/w0.rs b/src/nodes/lib/bank-hash/tests/w0.rs new file mode 100644 index 00000000..0665a309 --- /dev/null +++ b/src/nodes/lib/bank-hash/tests/w0.rs @@ -0,0 +1,85 @@ +use bebop_bank_hash::{compare_offline, BTraceBank, BTraceRecord, BTraceSource, BTraceTime, CompareResult}; + +fn record(source: BTraceSource, hart_id: u64, inst_id: u64, vbank_id: u32, hash: u32) -> BTraceRecord { + BTraceRecord::new( + source, + inst_id, + hart_id, + BTraceBank { vbank_id, hash }, + 33, + "mvin", + BTraceTime::Cycle(1), + None, + None, + ) +} + +#[test] +fn compares_only_the_matching_hart_and_instruction() { + let subject = [ + record(BTraceSource::Rtl, 0, 7, 4, 11), + record(BTraceSource::Rtl, 1, 7, 5, 12), + ]; + let golden = [ + record(BTraceSource::Bemu, 1, 7, 5, 12), + record(BTraceSource::Bemu, 0, 7, 4, 11), + ]; + let comparisons = compare_offline(subject, golden); + assert_eq!(comparisons.len(), 2); + assert!(comparisons + .iter() + .all(|comparison| comparison.result == CompareResult::Pass)); + let diff = serde_json::to_value(&comparisons[0]).unwrap(); + assert!(diff["subject"].is_object()); + assert!(diff["golden"].is_object()); +} + +#[test] +fn detects_wrong_hash_bank_and_unmatched_records() { + let subject = [ + record(BTraceSource::Rtl, 0, 1, 4, 11), + record(BTraceSource::Rtl, 0, 2, 4, 11), + record(BTraceSource::Rtl, 0, 3, 4, 11), + record(BTraceSource::Rtl, 0, 4, 4, 11), + ]; + let golden = [ + record(BTraceSource::Bemu, 0, 1, 4, 12), + record(BTraceSource::Bemu, 0, 2, 5, 11), + record(BTraceSource::Bemu, 0, 5, 4, 11), + ]; + let comparisons = compare_offline(subject, golden); + assert_eq!( + comparisons.iter().map(|item| item.result).collect::>(), + [ + CompareResult::Mismatch, + CompareResult::Mismatch, + CompareResult::MissingGolden, + CompareResult::MissingGolden, + CompareResult::MissingSubject, + ] + ); +} + +#[test] +#[should_panic(expected = "duplicate BTrace record")] +fn rejects_duplicate_records() { + compare_offline( + [ + record(BTraceSource::Rtl, 0, 7, 4, 11), + record(BTraceSource::Rtl, 0, 7, 4, 12), + ], + [], + ); +} + +#[test] +fn rejects_legacy_three_bank_record() { + let mut value = serde_json::to_value(record(BTraceSource::Rtl, 0, 7, 4, 11)).unwrap(); + assert_eq!(value["hart_id"], 0); + assert_eq!(value["inst_id"], 7); + assert_eq!(value["w0"], serde_json::json!({"vbank_id": 4, "hash_u32": 11})); + assert!(value.get("r0").is_none()); + assert!(value.get("r1").is_none()); + value["r0"] = serde_json::json!({"vbank_id": 4294967295u32, "hash_u32": 0}); + assert!(serde_json::from_value::(value).is_err()); +} diff --git a/src/nodes/lib/rtl-trace/src/banktrace.rs b/src/nodes/lib/rtl-trace/src/banktrace.rs index 00d7bdc8..97923a74 100644 --- a/src/nodes/lib/rtl-trace/src/banktrace.rs +++ b/src/nodes/lib/rtl-trace/src/banktrace.rs @@ -22,13 +22,11 @@ pub fn init(log_dir: &Path, enabled: bool) -> io::Result<()> { Ok(()) } -pub fn btrace(inst_id: u64, hart_id: u64, r0: BTraceBank, r1: BTraceBank, w0: BTraceBank) { +pub fn btrace(inst_id: u64, hart_id: u64, w0: BTraceBank) { let record = BTraceRecord::new( BTraceSource::Rtl, inst_id, hart_id, - r0, - r1, w0, 0, "btrace".to_string(), diff --git a/src/nodes/lib/rtl-trace/src/dpi.rs b/src/nodes/lib/rtl-trace/src/dpi.rs index 30b4195f..c1eba33e 100644 --- a/src/nodes/lib/rtl-trace/src/dpi.rs +++ b/src/nodes/lib/rtl-trace/src/dpi.rs @@ -9,7 +9,7 @@ pub(crate) fn force_link() { let _ = dpi_itrace as extern "C" fn(u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32); let _ = dpi_mtrace as extern "C" fn(u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32, u32); - let _ = dpi_btrace as extern "C" fn(u32, u32, u32, u32, u32, u32, u32, u32, u32, u32); + let _ = dpi_btrace as extern "C" fn(u32, u32, u32, u32, u32, u32); let _ = dpi_pmctrace as extern "C" fn(u32, u32, u32, u32); let _ = dpi_mem_pmctrace as extern "C" fn(u32, u32, u32, u32); let _ = dpi_ctrace as extern "C" fn(u32, u32, u32, u32, u32, u32, u32, u32); @@ -100,10 +100,6 @@ pub extern "C" fn dpi_btrace( inst_id_hi: u32, hart_id_lo: u32, hart_id_hi: u32, - r0_vbank: u32, - r0_hash: u32, - r1_vbank: u32, - r1_hash: u32, w0_vbank: u32, w0_hash: u32, ) { @@ -111,14 +107,6 @@ pub extern "C" fn dpi_btrace( crate::banktrace::btrace( u64_from_words(inst_id_lo, inst_id_hi), u64_from_words(hart_id_lo, hart_id_hi), - bebop_bank_hash::BTraceBank { - vbank_id: r0_vbank, - hash: r0_hash, - }, - bebop_bank_hash::BTraceBank { - vbank_id: r1_vbank, - hash: r1_hash, - }, bebop_bank_hash::BTraceBank { vbank_id: w0_vbank, hash: w0_hash,