coprocessor generator #280
Conversation
There was a problem hiding this comment.
Getting to a good direction! Is this your first PR ever?
Please split this to multiple PRs with logical entities. For example, the FUGen could be submitted first and then the RISC-V support. Huge single commit PRs are difficult to review.
Also check the C++ style and comments. For example some of the classes/Types naming could be improved for readability, there are formatting issues (please check the clang formatter script and improve if needed).
Also a good habit is to describe what the PR brings in and why in the pull request comment.
|
|
||
| for (auto op : Ops_) { | ||
| if (op.first == operation) { | ||
| encF3 = (op.second & maskF3) >> 7; |
There was a problem hiding this comment.
Can you use RISCVTools here so we don't duplicate the encoding logic?
| protected: | ||
| // Finding RISCV custom Operations as in RISCVTDGEN | ||
| void | ||
| findCustomOps() { |
There was a problem hiding this comment.
This functionality seems to be shared in RISCVTDGen. In this case, it should be moved to the RISCVTools class so we don't get duplicate code.
|
|
||
| // RISCV format finder as in RISCVTDGEN | ||
| InstructionFormat* | ||
| findFormat(const std::string name) const { |
| @@ -951,7 +989,7 @@ namespace HDLGenerator { | |||
| } else if (lang == Language::Verilog) { | |||
| stream << "\n"; | |||
| stream << StringTools::indent(level) << "// " << name() << "\n"; | |||
| stream << StringTools::indent(level) << "always @*"; | |||
| stream << StringTools::indent(level) << "always_comb"; | |||
There was a problem hiding this comment.
Is always_comb also supported by plain verilog?
| assert(bem_ != NULL); | ||
| findCustomOps(); | ||
| initializeBackendContents(); | ||
| } | ||
|
|
||
| std::string |
There was a problem hiding this comment.
These methods should be removed. They are not used and have crept in somehow to this commit.
| std::cout << "Usage: riscv-tdgen" << std::endl | ||
| << " -o Output directory." << std::endl | ||
| << " -a ADF path." << std::endl; | ||
| << " -a ADF path." << std::endl | ||
| << " -r 'T':Enable ROCC encodings, OR 'F'" << std::endl; |
There was a problem hiding this comment.
Can you add a default value to this? We want to run this command from the command line, and setting the coprocessor encoding type each time should not be necessary.
| using std::string; | ||
| using std::vector; | ||
|
|
||
| int const DEFAULT_IMEMWIDTH_IN_MAUS = 1; |
| LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING | ||
| FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER | ||
| DEALINGS IN THE SOFTWARE. | ||
| Copyright (C) 2025 Tampere University. |
There was a problem hiding this comment.
We should not change the license of old files
| @@ -1,25 +1,20 @@ | |||
| /* | |||
| Copyright (c) 2002-2009 Tampere University. | |||
There was a problem hiding this comment.
Same here. Just use the old license.
openasip/src/applibs/HWGen/FUGen.hh
Outdated
| int config_width_= 17; //17 bits for the config bits | ||
| int input_opcodeWidth_= opcodeWidth_ + config_width_; | ||
| int config_width_ = 17; // 17 bits for the config bits | ||
| int input_opcodeWidth_ = opcodeWidth_ + config_width_; |
There was a problem hiding this comment.
Check the naming. C++ TCE style: inputOpcodeWidth_ etc.
openasip/src/applibs/HWGen/FUGen.hh
Outdated
| //CVXIF generator enable | ||
| bool cvxifEN_= false; | ||
| // CVXIF generator enable | ||
| bool cvxifEN_ = false; |
There was a problem hiding this comment.
| bool cvxifEN_ = false; | |
| bool generateCVXIF_ = false; |
| @@ -0,0 +1,120 @@ | |||
| // Copyright (c) 2025 Tampere University of Technology. | |||
No description provided.