Calyx AXI: implementation lacks conversion between XRT address and Calyx memory addresses
Closed this issue · 2 comments
The current AXI implementation manages to inadvertently convert between byte-addressable base_address
addresses and WIDTH
-addressable curr_address
addresses because we assume the initial base addresses are 0 in our MVP. However I believe the XRT spec requires base addresses to start at 0x1000
(maybe 0x2000? Either way I recall that it is non-zero).
The current implementation has a read-channel curr_addr
which indexes into calyx memories like so:
cells {
ref curr_addr = std_reg(64);
}
// stuff ...
some_group {
calyx_mem.addr0 = curr_addr.out;
}
And in the main component we have groups that set curr_addr
to the value of base_addr
:
group set_curr_to_base_addr_A0{
curr_addr_A0.in = base_addr_A0.out;
curr_addr_A0.write_en = 1'b1;
set_curr_to_base_addr_A0[done] = curr_addr_A0.done;
}
where base_addr
is related to kernel xml files in a way that I don't fully understand at the moment, and drives xADDR
signals:
ARADDR = base_addr.out;
In general the value of base_addr != the value of curr_addr at the start of a transaction. So we need some method of converting between byte addressed base_address
and WIDTH
addressed curr_addr.
. This feels like it may be expensive? I talk about this more in this thread.
It's good to note that for our current load -> compute -> store scheme, we can bypass this issue by having separate counters for our base address and calyx memories. But in the future, if we want to allow for dynamic loads/stores or load more than 256 chunks of data we will need a way to convert between the two.
Closing this for now as #1854 solves this immediate issue. May be useful to keep this in mind when we work on a wrapper that allows for dynamic AXI requests, but it seems like there is a general way forward where we round up to the next largest power of 2 to avoid complicated mult/div work and instead just shift things around, if necessary.
Thanks for the clear explanation here! As I'm just catching up, I want to note that there is another route for addressing this specific problem:
So we need some method of converting between byte addressed
base_address
andWIDTH
addressedcurr_addr.
. This feels like it may be expensive?
To put it in pseudo-C terms, I think what you're saying is that we might need to do something like this:
OUT_TYPE read_value(int curr_addr) {
int byte_offset = (curr_addr * WIDTH) / 8; // assuming everything divides nicely
return axi_read(byte_offset + BASE_ADDR);
}
Is that an accurate reflection? Anyway, it's worth keeping in mind that, if we are actually doing this in a loop to load an entire memory or whatever, like this:
void load_entire_memory(OUT_TYPE *mem) {
for (int i = 0; i < LENGTH; ++i) {
mem[i] = read_value(i);
}
}
Then you can do a kind of "induction variable elimination" and get rid of the *
and /
altogether, replacing them with +=
bumps:
int axi_addr = BASE_ADDR;
int byte_width = WIDTH / 8; // again assuming nice division here
for (int i = 0; i < LENGTH; ++i) {
axi_addr += byte_width;
mem[i] = axi_read(axi_addr);
}
I realize this may be a little cryptic, but hopefully close to making sense?