-
Notifications
You must be signed in to change notification settings - Fork 78
feat: add SystemVerilog backend generator #1054
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,179 @@ | ||||||
| #!/usr/bin/env python3 | ||||||
|
|
||||||
| import argparse | ||||||
| import os | ||||||
| import sys | ||||||
| import logging | ||||||
| from pathlib import Path | ||||||
|
|
||||||
| # Add parent directory to path for imports | ||||||
| sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) | ||||||
|
|
||||||
| from generator import load_instructions, load_csrs | ||||||
|
|
||||||
|
|
||||||
| def format_instruction_name(name): | ||||||
| """Format instruction name for SystemVerilog (uppercase with underscores).""" | ||||||
| # Handle compressed instructions | ||||||
| if name.startswith("c."): | ||||||
| name = "C_" + name[2:] | ||||||
| # Replace dots with underscores and convert to uppercase | ||||||
| return name.replace(".", "_").upper() | ||||||
|
|
||||||
|
|
||||||
| def format_csr_name(name): | ||||||
| """Format CSR name for SystemVerilog (uppercase with underscores).""" | ||||||
| return "CSR_" + name.replace(".", "_").upper() | ||||||
|
|
||||||
|
|
||||||
| def match_to_sverilog_bits(match_str, is_compressed=False): | ||||||
| """Convert a match string to SystemVerilog bit pattern.""" | ||||||
| if not match_str: | ||||||
| return "32'b" + "?" * 32 | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this throw an error/warning instead of silently generating a match string of all wildcards that will match any instruction? That would be very problematic in use because it might shadow an actual match later. |
||||||
|
|
||||||
| # For compressed instructions (16-bit), we need to handle them differently | ||||||
| # The riscv-opcodes format puts the 16-bit pattern in the lower 16 bits | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably better for comments to stand alone instead of justifying choices based on riscv-opcodes. |
||||||
| # with the upper 16 bits as wildcards | ||||||
| if is_compressed or len(match_str) == 16: | ||||||
| # Pad with wildcards on the left for 16-bit instructions | ||||||
| match_str = "?" * 16 + match_str | ||||||
| elif len(match_str) < 32: | ||||||
| # For other cases, pad on the right | ||||||
| match_str = match_str + "-" * (32 - len(match_str)) | ||||||
|
Comment on lines
+40
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What cases do you envision this matching? If none, we're probably better off throwing an error here. |
||||||
|
|
||||||
| # Convert to SystemVerilog format (0, 1, or ?) | ||||||
| result = [] | ||||||
| for bit in match_str: | ||||||
| if bit == "0": | ||||||
| result.append("0") | ||||||
| elif bit == "1": | ||||||
| result.append("1") | ||||||
| else: # '-' or any other character | ||||||
| result.append("?") | ||||||
|
Comment on lines
+46
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this whole block be simplified to just |
||||||
|
|
||||||
| return "32'b" + "".join(result) | ||||||
|
|
||||||
|
|
||||||
| def generate_sverilog(instructions, csrs, output_file): | ||||||
| """Generate SystemVerilog package file.""" | ||||||
| with open(output_file, "w") as f: | ||||||
| # Write header | ||||||
| f.write("\n/* Automatically generated by parse_opcodes */\n") | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this is more clear?
Suggested change
|
||||||
| f.write("package riscv_instr;\n") | ||||||
|
|
||||||
| # Find the maximum name length for alignment | ||||||
| max_instr_len = max((len(format_instruction_name(name)) for name in instructions.keys()), default=0) | ||||||
| max_csr_len = max((len(format_csr_name(csrs[addr])) for addr in csrs.keys()), default=0) | ||||||
| max_len = max(max_instr_len, max_csr_len) | ||||||
|
|
||||||
| # Write instruction parameters | ||||||
| for name in sorted(instructions.keys()): | ||||||
| encoding = instructions[name] | ||||||
| sv_name = format_instruction_name(name) | ||||||
| # Pad the name for alignment | ||||||
| padded_name = sv_name.ljust(max_len) | ||||||
|
|
||||||
| # Get the match pattern | ||||||
| if isinstance(encoding, dict) and "match" in encoding: | ||||||
| match = encoding["match"] | ||||||
| else: | ||||||
| # If no match field, use all wildcards | ||||||
| match = "-" * 32 | ||||||
|
Comment on lines
+79
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above, please don't generate all wildcards if we don't find a match. A warning/error seems like a better choice here. |
||||||
|
|
||||||
| # Check if this is a compressed instruction | ||||||
| is_compressed = name.startswith("c.") | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the |
||||||
| sv_bits = match_to_sverilog_bits(match, is_compressed) | ||||||
| f.write(f" localparam [31:0] {padded_name} = {sv_bits};\n") | ||||||
|
|
||||||
| # Write CSR parameters | ||||||
| # CSRs are returned as {address: name} by load_csrs | ||||||
| for addr in sorted(csrs.keys()): | ||||||
| csr_name = csrs[addr] | ||||||
| sv_name = format_csr_name(csr_name) | ||||||
| # Pad the name for alignment | ||||||
| padded_name = sv_name.ljust(max_len) | ||||||
|
|
||||||
| # Format CSR address as 12-bit hex | ||||||
| f.write(f" localparam logic [11:0] {padded_name} = 12'h{addr:03x};\n") | ||||||
|
|
||||||
| # Write footer | ||||||
| f.write("\nendpackage\n") | ||||||
|
|
||||||
|
|
||||||
| def parse_args(): | ||||||
| parser = argparse.ArgumentParser( | ||||||
| description="Generate SystemVerilog package from RISC-V instruction definitions" | ||||||
| ) | ||||||
| parser.add_argument( | ||||||
| "--inst-dir", | ||||||
| default="../../../gen/resolved_spec/_/inst/", | ||||||
| help="Directory containing instruction YAML files", | ||||||
| ) | ||||||
| parser.add_argument( | ||||||
| "--csr-dir", | ||||||
| default="../../../gen/resolved_spec/_/csr/", | ||||||
| help="Directory containing CSR YAML files", | ||||||
| ) | ||||||
| parser.add_argument( | ||||||
| "--output", | ||||||
| default="inst.sverilog", | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about something like this instead?
Suggested change
|
||||||
| help="Output SystemVerilog file name" | ||||||
| ) | ||||||
| parser.add_argument( | ||||||
| "--extensions", | ||||||
| default="A,D,F,I,M,Q,Zba,Zbb,Zbs,S,System,V,Zicsr,Smpmp,Sm,H,U,Zicntr,Zihpm,Smhpm", | ||||||
| help="Comma-separated list of enabled extensions. Default includes standard extensions.", | ||||||
| ) | ||||||
|
Comment on lines
+122
to
+126
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should not default to a subset of (seemingly arbitrary) extensions. Not sure what the comments means when it says it defaults to the standard extensions. Similar to the generated C header, the default should be to include all extensions. |
||||||
| parser.add_argument( | ||||||
| "--arch", | ||||||
| default="RV64", | ||||||
| choices=["RV32", "RV64", "BOTH"], | ||||||
| help="Target architecture (RV32, RV64, or BOTH). Default is RV64.", | ||||||
| ) | ||||||
|
Comment on lines
+127
to
+132
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The generated C header does not have this option. I think it makes sense to keep the same CLI interface for both of them. The default output should also include everything, not just RV64. |
||||||
| parser.add_argument( | ||||||
| "--verbose", "-v", action="store_true", help="Enable verbose logging" | ||||||
| ) | ||||||
| parser.add_argument( | ||||||
| "--include-all", | ||||||
| action="store_true", | ||||||
| help="Include all instructions and CSRs regardless of extensions", | ||||||
| ) | ||||||
| return parser.parse_args() | ||||||
|
|
||||||
|
|
||||||
| def main(): | ||||||
| args = parse_args() | ||||||
|
|
||||||
| # Set up logging | ||||||
| log_level = logging.DEBUG if args.verbose else logging.INFO | ||||||
| logging.basicConfig(level=log_level, format="%(levelname)s:: %(message)s") | ||||||
|
|
||||||
| # Parse extensions | ||||||
| if args.include_all: | ||||||
| enabled_extensions = [] | ||||||
| logging.info("Including all instructions and CSRs (ignoring extension filter)") | ||||||
| else: | ||||||
| enabled_extensions = [ext.strip() for ext in args.extensions.split(",")] | ||||||
| logging.info(f"Enabled extensions: {', '.join(enabled_extensions)}") | ||||||
|
|
||||||
| logging.info(f"Target architecture: {args.arch}") | ||||||
|
|
||||||
| # Load instructions | ||||||
| instructions = load_instructions( | ||||||
| args.inst_dir, enabled_extensions, args.include_all, args.arch | ||||||
| ) | ||||||
| logging.info(f"Loaded {len(instructions)} instructions") | ||||||
|
|
||||||
| # Load CSRs | ||||||
| csrs = load_csrs(args.csr_dir, enabled_extensions, args.include_all, args.arch) | ||||||
| logging.info(f"Loaded {len(csrs)} CSRs") | ||||||
|
|
||||||
| # Generate the SystemVerilog file | ||||||
| generate_sverilog(instructions, csrs, args.output) | ||||||
| logging.info( | ||||||
| f"Generated {args.output} with {len(instructions)} instructions and {len(csrs)} CSRs" | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| if __name__ == "__main__": | ||||||
| main() | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like there is no reason to special-case compressed instructions here? The
c.-->C_conversion should be handled by the standardname.replace(".", "_").upper().