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 calyx-py AXI generator address channels #1855

Merged
merged 105 commits into from
Jan 23, 2024

Conversation

nathanielnrn
Copy link
Contributor

@nathanielnrn nathanielnrn commented Jan 19, 2024

This PR contains code for the generation of AR and AW channels that correspond to the linked hard coded AXI implementation. This is parameterized where applicable, although there are still some outstanding todos regarding this topic.

I have attempted to check (manually) that this exactly corresponds with the hardcoded AXI implementation (barring differences in ordering and perhaps some width differences resulting from parameterization), as that is known to pass some basic tests. But it is possible I missed something.

Note to see the output of the AR channel add_arread_channel(prog, mems[0]) must be uncommented in the build() function.

nathanielnrn and others added 30 commits December 22, 2023 11:56
TBD if this actually implements AXI correctly.

There are currently some hacks in place (marked with todos)
to get this to compile, namely some splicing
that doesn't consider what we actually want to splice
(it just takes [31:0]) as opposed to dynamically considering actual
bits we want.

A few other things that should be cleaned up eventually

Need to create a cocotb testbench to test correctness
Maybe this shouldn't be here, but for now (having deleted my
working directory earlier) putting it here
Simply run make from the cocotb directory and axi-read-tests
will be executed
We tie ARID low in our manager
Prefixes should not contain trailing "_"
Got rid of "assert_val" and "block_transfer" groups
and instead perform these things inside "do_ar_transfer", this is
required because we cant assert valid before we drive the data
correctly, so needs to happen in parallel.

Currently: This seems to write 16 times to same place, this is due to
hardcoding of 16 in ar transfer, not sure why address doesn't
increment this is tbd (and next TODO)
This is part of read channel control sequence
Also reduces data bus width to 32
Copy link
Contributor

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

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

WOW! This is incredibly cool!! I checked things over statically (i.e., I haven't run the generator yet) and everything looks awesome. I am THRILLED that the builder made this code relatively short… at least it did not, like, multiply the lines of code vs. the original Calyx version. I think this is good to go!

As a low priority, it would be interesting to get your qualitative impressions about how this porting went:

  • Was the translation from "real Calyx" to "Calyx embedded in Python" as tedious as I warned?
  • Was there anything that was particularly error prone when doing the translation? (Or I guess, can you think of anything you wish the builder library did for you that it does not?)
  • Are there any nice things about doing the porting this way (aside from the actual goal of parameterization)? For example, easy ways to reduce duplication, or whatever.

@nathanielnrn nathanielnrn merged commit 0dd9bc3 into main Jan 23, 2024
7 checks passed
@nathanielnrn nathanielnrn deleted the py-axi-generator-address-channels branch January 23, 2024 15:41
rachitnigam added a commit that referenced this pull request Feb 16, 2024
* init commit of hardcoded axi wrapper for a 'main' kernel

* add axi-reads-calix

* hook up inputs to channels in the wrapper. tbd if this works

* Working calyx verison of AR and R

TBD if this actually implements AXI correctly.

There are currently some hacks in place (marked with todos)
to get this to compile, namely some splicing
that doesn't consider what we actually want to splice
(it just takes [31:0]) as opposed to dynamically considering actual
bits we want.

A few other things that should be cleaned up eventually

Need to create a cocotb testbench to test correctness

* Track output of compiled calyx read channel

Maybe this shouldn't be here, but for now (having deleted my
working directory earlier) putting it here

* update gitignore to get rid of sim_build and other cocotb artifacts

* Working make files  for running cocotb tests

Simply run make from the cocotb directory and axi-read-tests
will be executed

* Add xID signals for cocotb compatability

We tie ARID low in our manager

* Fix prefix issue on cocotb axi test bench

Prefixes should not contain trailing "_"

* commit to repro 'make WAVES=1' cocotb error from axi-reads-calyx.futil

* axi-reads patch

* sync debug

* Add txn_len initialization to 16 in calyx program

* AXI Read fixed to get to read channel start

Got rid of "assert_val" and "block_transfer" groups
and instead perform these things inside "do_ar_transfer", this is
required because we cant assert valid before we drive the data
correctly, so needs to happen in parallel.

Currently: This seems to write 16 times to same place, this is due to
hardcoding of 16 in ar transfer, not sure why address doesn't
increment this is tbd (and next TODO)

* Add integer byte conversion for tests on Calyx AXI testharness

* WIP get reads to work. Add incr_curr_addr group

This is part of read channel control sequence

* remove .fst from tracking

* Add more data to testbench to make waveform viewing easier

* Reads seem to be terminating correctly at RLAST

* AR transfers seem to work, valid is high for 1 cycle

* Unreduced axi-reads-calyx.futil

Also reduces data bus width to 32

* Cocotb testbench now passes

* Formatted and passing axi-read-tests

* Reduce and comment axi-reads-calyx.futil

* remove axi-reads.v from being tracked

* add a todo

* add required ARPROT signal. This is hardcoded to be priviliged

* rename directories to yxi/axi-calyx

* initial commit of axi-writes-calyx, a copy of axi-reads-calyx

* WIP axi writes

* rename directories

* WIP imlpementing writes

* add testing for writes, note makefile is overwritten so now tests writes, not reads

* passing axi writes and testing

* init commit of hardcoded axi wrapper for a 'main' kernel

* add axi-reads-calix

* hook up inputs to channels in the wrapper. tbd if this works

* Working calyx verison of AR and R

TBD if this actually implements AXI correctly.

There are currently some hacks in place (marked with todos)
to get this to compile, namely some splicing
that doesn't consider what we actually want to splice
(it just takes [31:0]) as opposed to dynamically considering actual
bits we want.

A few other things that should be cleaned up eventually

Need to create a cocotb testbench to test correctness

* Track output of compiled calyx read channel

Maybe this shouldn't be here, but for now (having deleted my
working directory earlier) putting it here

* Working make files  for running cocotb tests

Simply run make from the cocotb directory and axi-read-tests
will be executed

* Add xID signals for cocotb compatability

We tie ARID low in our manager

* Fix prefix issue on cocotb axi test bench

Prefixes should not contain trailing "_"

* commit to repro 'make WAVES=1' cocotb error from axi-reads-calyx.futil

* axi-reads patch

* sync debug

* Add txn_len initialization to 16 in calyx program

* AXI Read fixed to get to read channel start

Got rid of "assert_val" and "block_transfer" groups
and instead perform these things inside "do_ar_transfer", this is
required because we cant assert valid before we drive the data
correctly, so needs to happen in parallel.

Currently: This seems to write 16 times to same place, this is due to
hardcoding of 16 in ar transfer, not sure why address doesn't
increment this is tbd (and next TODO)

* Add integer byte conversion for tests on Calyx AXI testharness

* WIP get reads to work. Add incr_curr_addr group

This is part of read channel control sequence

* remove .fst from tracking

* Add more data to testbench to make waveform viewing easier

* Reads seem to be terminating correctly at RLAST

* AR transfers seem to work, valid is high for 1 cycle

* Unreduced axi-reads-calyx.futil

Also reduces data bus width to 32

* Cocotb testbench now passes

* Formatted and passing axi-read-tests

* Reduce and comment axi-reads-calyx.futil

* remove axi-reads.v from being tracked

* add a todo

* add required ARPROT signal. This is hardcoded to be priviliged

* rename directories to yxi/axi-calyx

* initial commit of axi-writes-calyx, a copy of axi-reads-calyx

* WIP axi writes

* rename directories

* WIP imlpementing writes

* add testing for writes, note makefile is overwritten so now tests writes, not reads

* passing axi writes and testing

* Work on full AXI wrapper, reads and compute works

* cleaned up combined futil and tests

* delete axi-reads* which is subsumed by axi-combined

* add axi-combined-tests.py

* remove axi-writes as it is subsumed by axi-combined

* formatting

* Update yxi/axi-calyx/axi-combined-calyx.futil

Co-authored-by: Adrian Sampson <asampson@cs.cornell.edu>

* formatting

* add sim.sh which goes from calyx to running tests

* simplify valid.in signals

* WIP: replace groups with reg invokes

* add python file that enables waveform (vcd/fst) generation

* formatting

* simplify valid.in signals

* WIP: replace groups with reg invokes

* Replaces register-init groups with invokes

* Formatting of invokes

* Replace reg groups with invokes in main

* Modify tests to account for base address != 0

* Separate base-address calyx-mem-address dependency

This solution, made for our load->compute->store scheme, simply
increments the base_addr and curr_addr differently.
This should make it easy to have multiple transactions, which this
hardcoded does not support

* move incrs into par block

* iitial axi-generator commit

* WIP get arread-channel working

* Finished ARREAD channel.

TODO: Compare two, look at getting binary built. Look at improving
*_use/modifying to fit needs better

* Create m_to_s_address_channel for {AR,AW} channels

* WIP: Add read channel

* Finished read_channel. Still need to fix #1850

* Finished read channels

* Remove read channel to break up into multiple PRs

---------

Co-authored-by: Rachit Nigam <rachit.nigam12@gmail.com>
Co-authored-by: Adrian Sampson <asampson@cs.cornell.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: FPGA Changes for the FPGA backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants