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

[RTL] Define modules which support extra channel signals when possible #140

Open
lucas-rami opened this issue Sep 2, 2024 · 6 comments
Open
Labels
feature About a new feature in Dynamatic major Key functionnality not part of the core flow

Comments

@lucas-rami
Copy link
Member

lucas-rami commented Sep 2, 2024

The type system redesign allows us to define dataflow channels with an arbitrary number of extra signals, each of which can have an arbitrary name, type, and direction (downstream or upstream). While the compilation flow supports these optional extra signals throughout, none of our RTL components' respective implementation supports them.

In the general case, such implementations are impossible to define as static generics since extra signals can vary in number and nature (though our --handshake-reshape-channels pass, which runs before lowering to HW, helps in combining extra signals together on some channels to reduce the number of possible channel "layouts" at the boundary of some operations). Therefore, the easiest solution is to generate components which can support many different channel layouts at their IO on-demand depending on the need of the circuit. While we could use Chisel for this, a simpler text-based and generic generator may be enough (I'm thinking something that sorts of acts as the C/C++ pre-processor, reading an input HDL file and using blocks such as #ifdef/#endif within it to conditionally include part of the implementation).

For example, here is the current VHDL interface for our addi component, which only works for input/output channels with no extra signal.

entity addi is
  generic (
    DATA_TYPE : integer
  );
  port (
    -- inputs
    clk          : in std_logic;
    rst          : in std_logic;
    lhs          : in std_logic_vector(DATA_TYPE - 1 downto 0);
    lhs_valid    : in std_logic;
    rhs          : in std_logic_vector(DATA_TYPE - 1 downto 0);
    rhs_valid    : in std_logic;
    result_ready : in std_logic;
    -- outputs
    result       : out std_logic_vector(DATA_TYPE - 1 downto 0);
    result_valid : out std_logic;
    lhs_ready    : out std_logic;
    rhs_ready    : out std_logic
  );
end entity;

We could imagine replacing it with the following to add support for an optional extra downstream signal named "mergedDown" (that would be the name given to the merged downstream signal by our --handshake-reshape-channels pass if the channel had at least one extra downstream signal).

#ifdef EXTRA_DOWN
entity addi_DATA_WIDTH_EXTRA_DOWN_WIDTH is
#else
entity addi_DATA_WIDTH is
#endif
  port (
#ifdef EXTRA_DOWN
    -- new signals
    lhs_mergedDown    : in std_logic_vector(EXTRA_DOWN_WIDTH - 1 downto 0);
    rhs_mergedDown    : in std_logic_vector(EXTRA_DOWN_WIDTH - 1 downto 0);
    result_mergedDown : out std_logic_vector(EXTRA_DOWN_WIDTH - 1 downto 0);
#endif
    -- inputs
    clk          : in std_logic;
    rst          : in std_logic;
    lhs          : in std_logic_vector(DATA_WIDTH - 1 downto 0);
    lhs_valid    : in std_logic;
    rhs          : in std_logic_vector(DATA_WIDTH - 1 downto 0);
    rhs_valid    : in std_logic;
    result_ready : in std_logic;
    -- outputs
    result       : out std_logic_vector(DATA_WIDTH - 1 downto 0);
    result_valid : out std_logic;
    lhs_ready    : out std_logic;
    rhs_ready    : out std_logic
  );
end entity;

Combining the conditional inclusion of code inside #ifdef blocks with simple text replacements (e.g., like in our rtl-text-generator), this would allow us to have a "single implementation" of the addi that works for both the simple case when no extra signal is present and for the case when any number of extra downstream signals are present (since these will always be merged together by our channel-reshaping pass before lowering to HW). Adding support in this same component for extra upstream signals should be easy once we can do it for downstream signals.

Not shown here is what the architecture of that component looks like. While there is no doubt that it can be written in the same way as the entity declaration using our "preprocessor macros", one would have to define how extra signals are actually handled by the circuit. Every component would have its own rules on what it can support and on how extra signals are carried from inputs to outputs.

@lucas-rami lucas-rami added minor Related to specific use cases only feature About a new feature in Dynamatic labels Sep 2, 2024
@lucas-rami lucas-rami mentioned this issue Sep 2, 2024
11 tasks
@lucas-rami lucas-rami added major Key functionnality not part of the core flow and removed minor Related to specific use cases only labels Sep 5, 2024
@murphe67
Copy link
Collaborator

murphe67 commented Dec 5, 2024

I was discussing this with @Jiahui17 and @shundroid today, but I had been imagining something much more like this-
image

it would involve adding these extra outputs of transfer in and transfer out to control the FIFO, but we would avoid touching the original RTL as much as possible. Having the wrapper written in an easy-to-read language would make generation of units as extensible as possible

@Jiahui17
Copy link
Collaborator

Jiahui17 commented Dec 5, 2024

This assumes that the baseOp's behavior is never influenced by the modifier, could we always make it like this? For instance, like the tag comparison thing (an adder only operates on data with the same tags)

@murphe67
Copy link
Collaborator

murphe67 commented Dec 5, 2024

I think this is a reasonable assumption but we should definitely talk to @AyaElAkhras.

For adders I believe the aligner is completely separate to the adder
Screenshot_20241205-191632

I think in general tags could alter handshaking behaviour, but that would still be possible- you have explicit access to all of the handshaking signals and can do what you want with them.

The assumption would instead be that between transfer in and transfer out no behavior is modified

@AyaElAkhras
Copy link
Member

AyaElAkhras commented Dec 5, 2024

For adders I believe the aligner is completely separate to the adder

Yes, Taggers, Aligners, and Untaggers are added outside of the dataflow components.

I'm clear on the description written earlier but I'm not 100% clear on @murphe67's diagram. Could you elaborate on what each box and the different edges represent?

I think in general tags could alter handshaking behaviour, but that would still be possible- you have explicit access to all of the handshaking signals and can do what you want with them.

I'm also not fully clear here. A tag can be thought of as extra bits concatenated to the data inputs and propagated to the data outputs. In case of multi-input units (e.g., an Adder), we simply pass to the output the tag of one of the inputs without further checking if the tags of the other inputs are consistent with the chosen one because we guarantee by construction that they should be always consistent. That said, the tags should not alter the handshaking behaviour.

@murphe67
Copy link
Collaborator

murphe67 commented Dec 5, 2024

I agree that currently we have no tags that alter handshaking behavior, only that with this implementation that they would retain the ability to.

The diagram is how speculation works for any long latency operation e.g. floating point arithmetic- the output speculation tag (purple) is calculated from or-ing the two inputs, and passed to a FIFO to allow pipelining.

The rest of the handshaking is standard data-valid-ready, and passed directly as is to the normal version of the operation i.e. the version without extra tags

@lucas-rami
Copy link
Member Author

Just to give my quick thoughts on this, I am perfectly fine with whatever is easiest to implement. I would only care that whatever solution one comes up with can apply to as many dataflow components as possible that "don't really care" for extra signals but need to forward them through somehow, as to reduce the amount of "almost the same code" duplication. Components which actually do something with these extra signals will probably have their own special generator, and that's fine by me as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature About a new feature in Dynamatic major Key functionnality not part of the core flow
Projects
None yet
Development

No branches or pull requests

4 participants