Skip to content

Commit

Permalink
hierarchy: derive signal type without factoring in the VHDL type
Browse files Browse the repository at this point in the history
The VHDL type has more userfacing semantic info, but it does
not always tell us exactly how the signal is encoded.
  • Loading branch information
ekiwi committed Jun 13, 2024
1 parent caf740b commit 7bff1ed
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 29 deletions.
23 changes: 10 additions & 13 deletions wellen/src/fst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,18 +182,6 @@ impl SignalWriter {
SignalType::BitVector(len) => {
let bits = len.get();

// nvc will declare boolean signals as 1-bit bit-vectors and then generate
// the strings "true" and "false"
let value = if value.len() > bits as usize && bits == 1 {
match value {
b"true" => b"1",
b"false" => b"0",
_ => value,
}
} else {
value
};

debug_assert_eq!(
value.len(),
bits as usize,
Expand Down Expand Up @@ -636,12 +624,21 @@ fn read_hierarchy<F: BufRead + Seek>(reader: &mut FstReader<F>) -> Result<Hierar
let name_id = h.add_string(var_name);
let type_name = type_name.map(|s| h.add_string(s));
let num_scopes = scopes.len();
// we derive the signal type from the fst tpe directly, the VHDL type should never factor in!
let signal_tpe = match tpe {
FstVarType::GenericString => SignalType::String,
FstVarType::Real
| FstVarType::RealTime
| FstVarType::RealParameter
| FstVarType::ShortReal => SignalType::Real,
_ => SignalType::from_uint(length),
};
h.add_array_scopes(scopes);
h.add_var(
name_id,
var_type,
signal_tpe,
convert_var_direction(direction),
length,
index,
SignalRef::from_index(handle.get_index()).unwrap(),
enum_type,
Expand Down
11 changes: 5 additions & 6 deletions wellen/src/ghw/hierarchy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,8 +1119,8 @@ fn add_var(
h.add_var(
name,
VarType::Enum,
crate::hierarchy::SignalType::from_uint(bits),
dir,
bits,
None,
signal_ref,
Some(enum_type),
Expand All @@ -1140,8 +1140,8 @@ fn add_var(
h.add_var(
name,
var_type,
crate::hierarchy::SignalType::from_uint(1),
dir,
1,
None,
signal_ref,
None,
Expand All @@ -1158,8 +1158,8 @@ fn add_var(
h.add_var(
name,
var_type,
crate::hierarchy::SignalType::from_uint(bits),
dir,
bits,
None,
signal_ref,
None,
Expand All @@ -1169,15 +1169,14 @@ fn add_var(
VhdlType::F64(_, maybe_range) => {
// TODO: we could use the range to deduce indices and tighter widths
let _range = FloatRange::from_f64_option(*maybe_range);
let bits = 64;
let index = read_signal_id(input, signals.max_signal_id())?;
let signal_ref = signals.register_scalar(index, SignalType::F64);
let var_type = VarType::Real;
h.add_var(
name,
var_type,
crate::hierarchy::SignalType::Real,
dir,
bits,
None,
signal_ref,
None,
Expand Down Expand Up @@ -1222,8 +1221,8 @@ fn add_var(
h.add_var(
name,
var_type,
crate::hierarchy::SignalType::from_uint(num_bits),
dir,
num_bits,
Some(range.as_var_index()),
signal_ref,
None,
Expand Down
9 changes: 1 addition & 8 deletions wellen/src/hierarchy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1053,8 +1053,8 @@ impl HierarchyBuilder {
&mut self,
name: HierarchyStringId,
tpe: VarType,
signal_tpe: SignalType,
direction: VarDirection,
raw_length: u32,
index: Option<VarIndex>,
signal_idx: SignalRef,
enum_type: Option<EnumTypeId>,
Expand All @@ -1075,13 +1075,6 @@ impl HierarchyBuilder {
}
self.handle_to_node[handle_idx] = Some(var_id);

// for strings, the length is always flexible
let signal_tpe = match tpe {
VarType::String => SignalType::String,
VarType::Real => SignalType::Real,
_ => SignalType::from_uint(raw_length),
};

// now we can build the node data structure and store it
let node = Var {
parent,
Expand Down
12 changes: 10 additions & 2 deletions wellen/src/vcd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,17 +251,25 @@ fn read_hierarchy(
}
};
let (var_name, index, scopes) = parse_name(name)?;
let raw_vcd_var_tpe = convert_var_tpe(tpe)?;
// we derive the signal type from the vcd var directly, the VHDL type should never factor in!
let signal_tpe = match raw_vcd_var_tpe {
VarType::String => SignalType::String,
VarType::Real | VarType::RealTime | VarType::ShortReal => SignalType::Real,
_ => SignalType::from_uint(length),
};
// combine the raw variable type with VHDL type attributes
let (type_name, var_type, enum_type) =
parse_var_attributes(&mut attributes, convert_var_tpe(tpe)?, &var_name)?;
parse_var_attributes(&mut attributes, raw_vcd_var_tpe, &var_name)?;
let name = h.add_string(var_name);
let type_name = type_name.map(|s| h.add_string(s));
let num_scopes = scopes.len();
h.add_array_scopes(scopes);
h.add_var(
name,
var_type,
signal_tpe,
VarDirection::vcd_default(),
length,
index,
id_to_signal_ref(id, var_count),
enum_type,
Expand Down
19 changes: 19 additions & 0 deletions wellen/tests/fst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,23 @@ fn test_nvc_vhdl_test_bool_issue_16() {
// turns out that NVC needs a little hack to deal with the way it encodes booleans
let mut waves = read("inputs/nvc/vhdl_test_bool_issue_16.fst").unwrap();
load_all_signals(&mut waves);

// check signal values
let h = waves.hierarchy();
let toplevel = h.first_scope().unwrap();
let var = h.get(toplevel.vars(h).next().unwrap());
assert_eq!(var.full_name(h), "test_bool.bool");
let time_and_values = waves
.get_signal(var.signal_ref())
.unwrap()
.iter_changes()
.map(|(time, value)| format!("{time} {}", value.to_string()))
.collect::<Vec<_>>();
assert_eq!(
time_and_values,
[
// the boolean is encoded as string literals by nvc
"0 false", "1 true", "2 false"
]
);
}

0 comments on commit 7bff1ed

Please sign in to comment.