[Backend] Refactor arithmetic units in VHDL beta backend #552
Conversation
|
@Jiahui17 This is ready to merge now :) |
| signal dout_r : std_logic_vector(32 - 1 downto 0); | ||
| begin | ||
| --------------------- Instantiation ----------------- | ||
| addf_vitis_hls_single_precision_lat_8_u : entity work.addf_vitis_hls_single_precision_lat_8 |
There was a problem hiding this comment.
Is the latency fixed for Vivado floating point IPs?
If so, I want to assert generate_vivado_ip_wrapper received a valid latency as the argument
| architecture = f""" | ||
| -- Architecture of mul_4_stage | ||
| architecture behav of {name} is | ||
| latency = params["latency"] |
There was a problem hiding this comment.
also, I want to add assert(latency == 4) here
| Operation *op = $_op.getOperation(); | ||
| if (auto attr = op->getAttrOfType<StringAttr>("internal_delay")) | ||
| return attr; | ||
| return StringAttr::get(op->getContext(), "0.0"); |
There was a problem hiding this comment.
If internal_delay is stored in the format of "0.0", do we need to convert it into "0_0" before passing to the RTL?
There was a problem hiding this comment.
I think this default value is wrong, the buffering pass sets the converted format:
So can you update this to "0_0"? (Also the comment above 'Defaults to "0.0" if not set.')
| # only used for flopoco | ||
| is_double = params.get("is_double", None) | ||
| internal_delay = params.get("internal_delay", None) |
There was a problem hiding this comment.
In addition to the backend, latency is also used in the buffer placement (there, we also care if the unit has a zero cycle latency). Should the latency interface also be used for that? Or this is just like for verifying if the op has annotated with a latency value
|
On a set note, I think it will be good to add some unittests on CI/CD just to test if everything is fine |
|
I had some smoke tests for the new solver api: https://github.com/EPFL-LAP/dynamatic/blob/main/unittests/Support/ConstraintProgramming/CPTest.cpp |
|
I just had some questions on the interface design |
|
Thank you for reading!
yes, i am running the normal CI/CD with this backend on the server, I want to switch the whole ci/cd to this (and remove the json for old backend) once this is merged. Andela is working on something for unit tests of individual unit generation, I think it is a next important goal also.
I should make a whole markdown file for how the backend handles units with IP cores but basically- the backend receives a set of "IP selecting parameters" and a latency. It generates the name of the IP core based on the first set of parameters, and then adds a "valid propagation buffer" with size based on the latency. The parameters for selecting the flopoco ip are different to selecting the vivado ip. The whole IP selection process should be improved, but the latency part is relatively final. |
If we dont pass the unit generators a latency, each generator must hardcode it for each pair (handshake op, IP selected), which is very fragile. Since the unit generators don't know the details of the IP, it doesn't make sense for them to know the latencies, and so they should recieve it as a parameter (how long should the valid propagation buffer be). The interface just stores the result from the components.json file for the (handshake op, IP selected) pair- if the buffering pass reads directly from the json, it doesnt need the interface. The buffering pass is also what writes to the interface currently. |
Oh I forgot this interface is not the same as the kv pair in the hw.parameter attribute of the unit. I think this design makes a lot of sense, we could do something similar for the delay characteristic as well (also, buffer pass should just use the interface to get the latency/delay). |
4 participants
In order to remove our need for 4 different backends, we should expand the capabilities of the beta backend to encompass all of them.
This PR changes how we generate all of our arithmetic units in the VHDL beta backend, such that it now contains all features of the current VHDL backend, including the work on floating point units by @Carmine50 and @KillianMcCourt.
It also converts our floating point subtraction unit to be a direct wrapper around our adder.