Separate fabric and tile configuration #154
Conversation
There was a problem hiding this comment.
We should keep the backward compatibility with the single CSV format.
There was a problem hiding this comment.
That I hadn't thought of that! Should now support the old fabric configuration format. I had to change parseTiles and parseSupertiles to return a list of objects.
There was a problem hiding this comment.
Rather than deprecation, we can introduce a new keyword, for example, TILE_PATH, to indicate a path is provided. Will also need to update the Doc to let people know this is available.
There was a problem hiding this comment.
The keywords that are used are Tile and Supertile to read the tile/supertile configuration from a path. Should I rename them?
I would keep the deprecation notice. It's not much work to move an existing fabric to the new format and the FABulous template uses the new format as well. Supporting both options is more work to maintain and I think it makes generally more sense when the configurations are split this way.
I would update the docs with the new way of describing tiles, if that's okay?
There was a problem hiding this comment.
Yes, this makes sense.
Let's keep the Tile and Supertile keywords since the TILE keyword will only happen in the "tile.csv".
Update the Doc with the new tile description method and mark deprecation for the current method. When the "2.0" release is in place, we can entirely remove the current way.
There was a problem hiding this comment.
I have now updated the docs to refer to the new method of tile description.
What I noticed: The example fabric uses the CPU_IO tile which is currently not available for Verilog and misses configurations in VHDL. I think it would be nice to have this available, should I open an issue regarding the missing files?
There was a problem hiding this comment.
Yes, open an issue about it. Someone is currently working on the automatic generation of the tile functionality. This should help with the situation.
There was a problem hiding this comment.
Done! I think this PR should be ready then.
mole99
force-pushed
the
separate-configs
branch
from
0771478
to
bfa4e4e
Compare
mole99
force-pushed
the
separate-configs
branch
from
bfa4e4e
to
e93b0e9
Compare
|
I think you will have other significant changes as well. We will need to maintain the current file structure for a tutorial shortly. I have started a new branch called |
|
Sounds good! I have changed the base branch to |
KelvinChung2000
merged commit bf7f1cf
into
FPGA-Research-Manchester:FABulous2.0-development
This PR separates the fabric and tile configuration. This has several benefits:
The fabric config only describes the fabric and the
Tile/folder contains all information about tilesThe git history for tile changes / fabric changes is separate
fabric.csvI carefully tried to not break anything and tested this PR with both the Verilog and VHDL writers:
load_fabricis successful for both of them. For Verilog all subsequent commands work as expected, for VHDLrun_FABulous_fabricthrows an error that is unrelated to this PR. See #153.Fixes #149.