Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ap_idle generation #1101

Merged
merged 6 commits into from
Jul 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion src/backend/xilinx/axi_address_space.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ pub(crate) struct Flags {
clear_on_read: Option<(AxiChannel, String)>,
/// Clear the internal register when there is a successful
/// handshake on this channel.
// XXX(nathanielnrn): might not be a good name? used for things that aren't just
// handshakes
clear_on_handshake: Option<String>,
/// Clear the internal register when ap_start is asserted
/// and invert `ARESET` logic (idle is high when reset)
idle: bool,
/// This register can be written to with the interface.
write: bool,
}
Expand Down Expand Up @@ -51,6 +56,11 @@ impl Flags {
self
}

/// Builder style function for setting the `idle` flag.
pub(crate) fn idle(mut self) -> Self {
self.idle = true;
self
}
/// Builder style function for setting the `write` flag.
pub(crate) fn write(mut self) -> Self {
self.write = true;
Expand Down Expand Up @@ -266,10 +276,12 @@ impl AddressSpace {
));

// XXX(sam) this is a hack to avoid iterating through the bit meanings again
// only happens for writes
let mut writes_exist: bool = false;
for meaning in addr.bit_meaning.iter().filter(|m| m.flags.write) {
// this part is only the assignment itself?? underneath the ARESET of the if branch
if_stmt.add_seq(v::Sequential::new_nonblk_assign(
self.slice(meaning),
self.slice(meaning), //gets name of register
v::Expr::new_int(0),
));
else_br.add_seq(v::Sequential::new_nonblk_assign(
Expand All @@ -294,8 +306,12 @@ impl AddressSpace {
}

// port writes to internal register logic
// reads only
// XXX(nathanielnrn) Why does this need to be seperate from the above write logic?
// Seems basically the same to me?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed; maybe this could be factored out into a function? Or maybe it would be too annoying because of the slight differences in flag values…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gave this a shot, indeed a bit annoying because of scope issues, the helper function has a bunch of arguments and imo is not much cleaner than how it is currently. I think refactoring this could be a small issue for later? Willing to work on this more though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, totally, makes sense to me! I can imagine how this would imply a broader refactoring that wouldn't be trivial.

for meaning in &addr.bit_meaning {
if let Some(port) = &meaning.flags.read {
//Takes in read string of Read option and assigns it to `port`
let mut branches = vec![
(Some("ARESET".into()), 0.into()),
(Some(port.as_str().into()), 1.into()),
Expand All @@ -308,6 +324,11 @@ impl AddressSpace {
);
branches.push((Some(cond), 0.into()));
}
if meaning.flags.idle {
branches[0] = (Some("ARESET".into()), 1.into());
let if_ap_start = v::Expr::new_ref("ap_start");
branches.push((Some(if_ap_start), 0.into()));
}
let always = super::utils::cond_non_blk_assign(
"ACLK",
self.slice(meaning),
Expand Down
9 changes: 7 additions & 2 deletions src/backend/xilinx/control_axi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fn axi_address_space(
AddressSpace::new(address_width, data_width)
.address(
0x0,
"AP_CONTROL",
"AP_CONTROL", //TODO: where does this disapear to?
vec![
(
0..1,
Expand All @@ -44,7 +44,12 @@ fn axi_address_space(
.read("ap_done")
.clear_on_read(axi.read_data.clone(), "raddr"),
),
// (2..3, "ap_idle", 0..1),,
(
2..3,
"int_ap_idle",
0..1,
Flags::default().read("ap_done").idle(),
),
],
)
.address(
Expand Down