From acb7608eb93b80bede7e432e8baf90476ad23658 Mon Sep 17 00:00:00 2001 From: Zapta Date: Mon, 14 Oct 2024 20:12:32 -0700 Subject: [PATCH 01/13] Added to the apio graph command a verification that the 'dot' command exists on the path. This is a command that the user is supposed to install manually. --- apio/commands/graph.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/apio/commands/graph.py b/apio/commands/graph.py index c53154b6..3f1faab8 100644 --- a/apio/commands/graph.py +++ b/apio/commands/graph.py @@ -8,6 +8,7 @@ """Implementation of 'apio graph' command""" from pathlib import Path +import shutil import click from click.core import Context from apio.managers.scons import SCons @@ -38,6 +39,14 @@ to the desired format using the dot command. """ +DOT_HELP = """ +The 'dot' command is part of the 'graphviz' suite. Please install +it per the instructions at https://graphviz.org/download and run +this command again. If you think that the 'dot' command is available +on the system path, you can try supressing this error message by +adding the --force flag to the apio graph command. +""" + @click.command( "graph", @@ -48,16 +57,38 @@ @click.pass_context @options.project_dir_option @options.top_module_option_gen(help="Set the name of the top module to graph.") +@options.force_option_gen(help="Force execution despite no 'dot' command.") @options.verbose_option def cli( ctx: Context, # Options project_dir: Path, + force: bool, verbose: bool, top_module: str, ): """Implements the apio graph command.""" + # -- This program requires a user install graphviz 'dot' available on + # -- the path. Verify it. + dot_path = shutil.which("dot") + if not dot_path: + if force: + # -- Just print a warning and continue. + click.secho( + "Warning: Skipping the check for the 'dot' command.", + fg="yellow", + ) + else: + # -- Print an error message and abort. + click.secho() + click.secho( + "Error: The 'dot' command was not found on the system path.", + fg="red", + ) + click.secho(DOT_HELP, fg="yellow") + ctx.exit(1) + # -- Crete the scons object scons = SCons(project_dir) From 46f04af0d5eeae89fb8bb6a13687dbf5b91e4984 Mon Sep 17 00:00:00 2001 From: Zapta Date: Sat, 19 Oct 2024 09:35:27 -0700 Subject: [PATCH 02/13] Added to the install/uninstall commands a --version flag. Also added a --sayyes flag to the uninstall command. No changed in behavior. --- apio/commands/install.py | 19 +++++++++--- apio/commands/options.py | 6 ++-- apio/commands/uninstall.py | 18 ++++++++--- apio/managers/installer.py | 63 ++++++++++++++++++++++++-------------- apio/profile.py | 6 +++- apio/resources.py | 4 +-- 6 files changed, 77 insertions(+), 39 deletions(-) diff --git a/apio/commands/install.py b/apio/commands/install.py index 2f61020c..24252477 100644 --- a/apio/commands/install.py +++ b/apio/commands/install.py @@ -19,19 +19,26 @@ def install_packages( - packages: list, platform: str, resources: Resources, force: bool + packages: list, + platform: str, + resources: Resources, + force: bool, + verbose: bool, ): """Install the apio packages passed as a list * INPUTS: - packages: List of packages (Ex. ['examples', 'oss-cad-suite']) - platform: Specific platform (Advanced, just for developers) - force: Force package installation + - verbose: Show detailed output. """ # -- Install packages, one by one... for package in packages: # -- The instalation is performed by the Installer object - modifiers = Installer.Modifiers(force=force, checkversion=True) + modifiers = Installer.Modifiers( + force=force, checkversion=True, verbose=verbose + ) installer = Installer(package, platform, resources, modifiers) # -- Install the package! @@ -71,6 +78,7 @@ def install_packages( @options.force_option_gen(help="Force the packages installation.") @options.project_dir_option @options.platform_option +@options.verbose_option def cli( ctx: Context, # Arguments @@ -81,6 +89,7 @@ def cli( force: bool, platform: str, project_dir: Path, + verbose: bool, ): """Implements the install command which allows to manage the installation of apio packages. @@ -94,13 +103,15 @@ def cli( # -- Install the given apio packages if packages: - install_packages(packages, platform, resources, force) + install_packages(packages, platform, resources, force, verbose) ctx.exit(0) # -- Install all the available packages (if any) if all_: # -- Install all the available packages for this platform! - install_packages(resources.packages, platform, resources, force) + install_packages( + resources.packages, platform, resources, force, verbose + ) ctx.exit(0) # -- List all the packages (installed or not) diff --git a/apio/commands/options.py b/apio/commands/options.py index d88d26e0..4491cce2 100644 --- a/apio/commands/options.py +++ b/apio/commands/options.py @@ -217,7 +217,7 @@ def top_module_option_gen( "-v", "--verbose", is_flag=True, - help="Show the entire output of the command.", + help="Show detailed output.", cls=cmd_util.ApioOption, ) @@ -226,7 +226,7 @@ def top_module_option_gen( "verbose_pnr", # Var name. "--verbose-pnr", is_flag=True, - help="Show the pnr output.", + help="Show detailed pnr output.", cls=cmd_util.ApioOption, ) @@ -235,6 +235,6 @@ def top_module_option_gen( "verbose_yosys", # Var name. "--verbose-yosys", is_flag=True, - help="Show the yosys output.", + help="Show detailed yosys output.", cls=cmd_util.ApioOption, ) diff --git a/apio/commands/uninstall.py b/apio/commands/uninstall.py index 0d0242bb..3b4032bb 100644 --- a/apio/commands/uninstall.py +++ b/apio/commands/uninstall.py @@ -19,17 +19,21 @@ from apio.commands import options -def _uninstall(packages: list, platform: str, resources: Resources): +def _uninstall( + packages: list, platform: str, resources: Resources, sayyes, verbose: bool +): """Uninstall the given list of packages""" # -- Ask the user for confirmation - if click.confirm("Do you want to uninstall?"): + if sayyes or click.confirm("Do you want to uninstall?"): # -- Uninstall packages, one by one for package in packages: # -- The uninstalation is performed by the Installer object - modifiers = Installer.Modifiers(force=False, checkversion=False) + modifiers = Installer.Modifiers( + force=False, checkversion=False, verbose=verbose + ) installer = Installer(package, platform, resources, modifiers) # -- Uninstall the package! @@ -71,6 +75,8 @@ def _uninstall(packages: list, platform: str, resources: Resources): @options.all_option_gen(help="Uninstall all packages.") @options.project_dir_option @options.platform_option +@options.sayyes +@options.verbose_option def cli( ctx: Context, # Arguments @@ -80,6 +86,8 @@ def cli( all_: bool, project_dir: Path, platform: str, + sayyes: bool, + verbose: bool, ): """Implements the uninstall command.""" @@ -91,7 +99,7 @@ def cli( # -- Uninstall the given apio packages if packages: - _uninstall(packages, platform, resources) + _uninstall(packages, platform, resources, sayyes, verbose) ctx.exit(0) # -- Uninstall all the packages @@ -99,7 +107,7 @@ def cli( # -- Get all the installed apio packages packages = Profile().packages # -- Uninstall them! - _uninstall(packages, platform, resources) + _uninstall(packages, platform, resources, sayyes, verbose) ctx.exit(0) # -- List all the packages (installed or not) diff --git a/apio/managers/installer.py b/apio/managers/installer.py index ef3c41f3..3386479f 100644 --- a/apio/managers/installer.py +++ b/apio/managers/installer.py @@ -34,13 +34,14 @@ class Modifiers: force: bool checkversion: bool + verbose: bool def __init__( self, package: str, platform: str = "", resources=None, - modifiers=Modifiers(force=False, checkversion=True), + modifiers=Modifiers(force=False, checkversion=True, verbose=False), ): """Class initialization. Parameters: * package: Package name to manage/install. It can have a sufix with @@ -79,6 +80,9 @@ def __init__( # -- Force installation or not self.force_install = modifiers.force + # -- Show detailed output. + self.verbose = modifiers.verbose + # -- Installer.package_dir: path were the packages are stored # -- Ex. /home/obijuan/.apio/packages self.packages_dir = "" @@ -116,20 +120,19 @@ def __init__( # Check if the version is ok (It is only done if the # checkversion flag has been activated) if modifiers.checkversion: - # Check version. The filename is read from the - # repostiroy - # -- Get the url of the version.txt file + # Check version. The filename is read from the repostiroy. + # -- Get the url of the version file url_version = data["release"]["url_version"] # -- Get the latest version # -- It will exit in case of error - valid_version = self._get_valid_version(url_version) + remote_version = self._get_latest_version(url_version) # -- It is only execute in case the version is valid # -- it will exit otherwise # Store the valid version - self.version = valid_version + self.version = remote_version # Get the plaform_os name # e.g., [linux_x86_64, linux] @@ -279,6 +282,9 @@ def _install_package(self, dlpath: Path): # -- Ex. '/home/obijuan/.apio/packages/examples' package_dir = self.packages_dir / self.package_name + if self.verbose: + click.secho(f"Package dir: {package_dir.absolute()}") + # -- Destination path is a folder (Already exist!) if package_dir.is_dir(): @@ -350,6 +356,9 @@ def uninstall(self): # -- Build the package filename file = self.packages_dir / self.package_name + if self.verbose: + click.secho(f"Package dir: {file.absolute()}") + # -- Check that it is a folder... if file.is_dir(): @@ -379,15 +388,15 @@ def _get_tarball_name(name, extension): tarball = f"{name}.{extension}" return tarball - def _get_valid_version(self, url_version: str) -> str: - """Get the latest valid version from the given remote - version.txt file. The file is downloaded and the version is + def _get_latest_version(self, url_version: str) -> str: + """Get the latest recommanded version from the given remote + version file. The file is downloaded and the version is read and returned - INPUTS: - * url_version: URL of the package's version.txt file + * url_version: URL of the package's version file Ex. https://github.com/FPGAwars/apio-examples/raw/master/ - version.txt + VERSION The url_version for every package is located in the file: resources/packages.json @@ -399,11 +408,13 @@ def _get_valid_version(self, url_version: str) -> str: # -- No checking... return the required version return self.version - # -- Find latest version number released - # -- It is found in the file version.txt located in the root folder of - # -- all the APIO packages + # -- Find latest version number released. It is found using the + # -- version url package configuration. if url_version: - # -- Get the version.txt with the latest version number + if self.verbose: + click.secho(f"Version url: {url_version}") + + # -- Get the version file with the latest version number req = requests.get(url_version, timeout=5) # -- Check the server response @@ -413,26 +424,26 @@ def _get_valid_version(self, url_version: str) -> str: == requests.codes.ok ): # -- Request OK - print("File version.txt downloaded!") + print("Remote version file downloaded.") # -- Read the version without the ending \n version = req.text.rstrip("\n") # -- Debug - print(f"Version: {version}") + print(f"Remote version: {version}") # -- Return the version return version # -- There was a problem with the request - click.secho("Error downloading the version.txt file", fg="red") + click.secho("Error downloading the version file", fg="red") click.secho(f"URL: {url_version}", fg="red") click.secho(f"Error code: {req.status_code}", fg="red") sys.exit(1) - # -- Error: No URL defined for the version.txt file + # -- Error: No URL defined for the version file click.secho( - "No URL defined for the version.txt file\n" + "No URL defined for the version file\n" + "It is not possible to get the latest version number", fg="red", ) @@ -449,11 +460,13 @@ def _download(self, url: str) -> str: """ # -- Check the installed version of the package - installed = self.profile.installed_version(self.package, self.version) + installed_ok = self.profile.is_installed_version_ok( + self.package, self.version, self.verbose + ) # -- Package already installed, and no force_install flag # -- Nothing to download - if installed and not self.force_install: + if installed_ok and not self.force_install: click.secho( f"Already installed. Version {self.version}", fg="yellow", @@ -461,6 +474,9 @@ def _download(self, url: str) -> str: return None # ----- Download the package! + if self.verbose: + click.secho(f"Src url: {url}") + # -- Object for downloading the file filed = FileDownloader(url, self.packages_dir) @@ -468,7 +484,8 @@ def _download(self, url: str) -> str: filepath = filed.destination # -- Inform the user - click.secho(f"Download {filed.fname}") + if self.verbose: + click.secho(f"Local file: {filepath}") # -- Download start! try: diff --git a/apio/profile.py b/apio/profile.py index ac225bc6..28b75981 100644 --- a/apio/profile.py +++ b/apio/profile.py @@ -7,6 +7,7 @@ import json from pathlib import Path +import click import semantic_version from apio import util @@ -32,7 +33,7 @@ def __init__(self): # -- Read the profile from file self.load() - def installed_version(self, name: str, version: str): + def is_installed_version_ok(self, name: str, version: str, verbose: bool): """Check the if the given package version is installed * INPUT: - name: Package name @@ -54,6 +55,9 @@ def installed_version(self, name: str, version: str): current_ver = semantic_version.Version(pkg_version) to_install_ver = semantic_version.Version(version) + if verbose: + click.secho(f"Current version: {current_ver}") + same_versions = current_ver == to_install_ver # -- Return the state of the installed package: diff --git a/apio/resources.py b/apio/resources.py index 6d4931a5..fc1e74cb 100644 --- a/apio/resources.py +++ b/apio/resources.py @@ -128,9 +128,7 @@ def _load_resource(self, name: str, allow_custom: bool = False) -> dict: if filepath.exists(): if allow_custom: - click.secho( - f"Info: Loading a custom '{name}' from the project dir." - ) + click.secho(f"Using project's custom '{name}' file.") return self._load_resource_file(filepath) # -- Load the stock resource file from the APIO package. From 07fb2d9d1c6ef948bf5d9641cd8ab9f74f6c3d59 Mon Sep 17 00:00:00 2001 From: Zapta Date: Sat, 19 Oct 2024 20:35:13 -0700 Subject: [PATCH 03/13] Added a new command 'apio report' . This command reports design timing and utilization. It will replace the 'apio time' command which reports only timing and is avilable only for ICE40. This commit include implementation for ice40. Will add implementations for ecp5 and gowin in followup commits. --- apio/__main__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/apio/__main__.py b/apio/__main__.py index 8094c7e5..500fb84a 100644 --- a/apio/__main__.py +++ b/apio/__main__.py @@ -82,6 +82,7 @@ def reformat_apio_help(original_help: str) -> str: "lint", "upload", "time", + "report", "graph", ], ) From 3686d95bd55c2c1b5a5018408237c6085035894a Mon Sep 17 00:00:00 2001 From: Zapta Date: Sat, 19 Oct 2024 20:36:19 -0700 Subject: [PATCH 04/13] Added a new command 'apio report' that prints a timing and utilization report. It will replace the 'apio time' command which prints only timing info. This command uses pnr timing reports rather than running a seperate timing analysis. This commit includes implementation for ice40. Gowin and ecp will be added in follow up commits. --- apio/commands/report.py | 88 +++++++++++++++++++++++++++++++++ apio/managers/scons.py | 19 +++++++- apio/scons/ice40/SConstruct | 34 ++++++++++--- apio/scons/scons_util.py | 95 ++++++++++++++++++++++++++++++------ test/commands/test_report.py | 46 +++++++++++++++++ test/commands/test_time.py | 2 + 6 files changed, 261 insertions(+), 23 deletions(-) create mode 100644 apio/commands/report.py create mode 100644 test/commands/test_report.py diff --git a/apio/commands/report.py b/apio/commands/report.py new file mode 100644 index 00000000..861b1953 --- /dev/null +++ b/apio/commands/report.py @@ -0,0 +1,88 @@ +# -*- coding: utf-8 -*- +# -- This file is part of the Apio project +# -- (C) 2016-2024 FPGAwars +# -- Authors +# -- * Jesús Arroyo (2016-2019) +# -- * Juan Gonzalez (obijuan) (2019-2024) +# -- Licence GPLv2 +"""Implementation of 'apio' report' command""" + +from pathlib import Path +import click +from click.core import Context +from apio.managers.scons import SCons +from apio import cmd_util +from apio.commands import options + + +# --------------------------- +# -- COMMAND +# --------------------------- +HELP = """ +The report command reports the utilization and timing of the design. +It is useful to analyzer utilization bottle neck and to verify that +the design can run at a desired clock speed. +The commands is typically used in the root directory +of the project that contains the apio.ini file. + +\b +Examples: + apio report + epio report --verbose +""" + + +# pylint: disable=duplicate-code +# pylint: disable=too-many-arguments +# pylint: disable=too-many-positional-arguments +@click.command( + "report", + short_help="Report design utilization and timing.", + help=HELP, + cls=cmd_util.ApioCommand, +) +@click.pass_context +@options.project_dir_option +@options.verbose_option +@options.top_module_option_gen(deprecated=True) +@options.board_option_gen(deprecated=True) +@options.fpga_option +@options.size_option +@options.type_option +@options.pack_option +def cli( + ctx: Context, + # Options + project_dir: Path, + verbose: bool, + top_module: str, + board: str, + fpga: str, + size: str, + type_: str, + pack: str, +): + """Analyze the design and report timing.""" + + # -- Create the scons object + scons = SCons(project_dir) + + # Run scons + exit_code = scons.report( + { + "board": board, + "fpga": fpga, + "size": size, + "type": type_, + "pack": pack, + "verbose": { + "all": False, + "yosys": False, + "pnr": verbose, + }, + "top-module": top_module, + } + ) + + # -- Done! + ctx.exit(exit_code) diff --git a/apio/managers/scons.py b/apio/managers/scons.py index 13c23ef5..4dc00256 100644 --- a/apio/managers/scons.py +++ b/apio/managers/scons.py @@ -231,8 +231,6 @@ def build(self, args) -> int: packages=["oss-cad-suite"], ) - # run(self, command, variables, packages, board=None, arch=None): - @on_exception(exit_code=1) def time(self, args) -> int: """Runs a scons subprocess with the 'time' target. Returns process @@ -258,6 +256,23 @@ def time(self, args) -> int: packages=["oss-cad-suite"], ) + @on_exception(exit_code=1) + def report(self, args) -> int: + """Runs a scons subprocess with the 'report' target. Returns process + exit code, 0 if ok.""" + + variables, board, arch = process_arguments( + args, self.resources, self.project + ) + + return self._run( + "report", + variables=variables, + board=board, + arch=arch, + packages=["oss-cad-suite"], + ) + @on_exception(exit_code=1) def upload(self, config: dict, prog: dict) -> int: """Runs a scons subprocess with the 'time' target. Returns process diff --git a/apio/scons/ice40/SConstruct b/apio/scons/ice40/SConstruct index 1d365eeb..1d09930a 100644 --- a/apio/scons/ice40/SConstruct +++ b/apio/scons/ice40/SConstruct @@ -65,6 +65,7 @@ from apio.scons.scons_util import ( make_waves_target, make_iverilog_action, make_verilator_action, + get_report_action, ) # -- Create the environment @@ -105,7 +106,7 @@ synth_srcs, test_srcs = get_source_files(env) PCF = get_constraint_file(env, ".pcf", TOP_MODULE) -# -- Apio build/upload. +# -- Apio build/upload/time/report. # -- Builder (yosys, Synthesis). # -- (modules).v -> hardware.json. synth_builder = Builder( @@ -120,17 +121,18 @@ synth_builder = Builder( env.Append(BUILDERS={"Synth": synth_builder}) -# -- Apio build/upload. +# -- Apio build/upload/time/report. # -- builder (nextpnr, Place and route). -# -- hardware.json -> hardware.asc. +# -- hardware.json -> hardware.asc, hardware.pnr. pnr_builder = Builder( action=( "nextpnr-ice40 --{0}{1} --package {2} --json $SOURCE --asc $TARGET " - "--pcf {3} {4}" + "--report {3}.pnr --pcf {4} {5}" ).format( FPGA_TYPE, FPGA_SIZE, FPGA_PACK, + TARGET, PCF, "" if VERBOSE_ALL or VERBOSE_PNR else "-q", ), @@ -139,6 +141,16 @@ pnr_builder = Builder( ) env.Append(BUILDERS={"PnR": pnr_builder}) +# -- Apio report. +# -- builder (fake, the .pnr file is generated by pnr_builder) +# -- hardware.asc -> hardware.pnr. +pnr_builder = Builder( + action="", + suffix=".pnr", + src_suffix=".asc", +) +env.Append(BUILDERS={"PnrReport": pnr_builder}) + # -- Apio build/upload. # -- Builder (icepack, bitstream generator). @@ -162,7 +174,7 @@ time_rpt_builder = Builder( env.Append(BUILDERS={"Time": time_rpt_builder}) -# -- Apio build/upload. +# -- Apio build/upload/time/report. # -- Targets. # -- (module).v -> hardware.json -> hardware.asc -> hardware.bin. synth_target = env.Synth(TARGET, [synth_srcs]) @@ -177,11 +189,20 @@ if VERBOSE_PNR: if VERBOSE_ALL: AlwaysBuild(synth_target, pnr_target, build_target) +# -- Apio report. +# -- Targets. +# -- hardware.asd -> hardware.pnr -> (report) +pnr_report_target = env.PnrReport(TARGET, pnr_target) +report_target = env.Alias( + "report", pnr_report_target, get_report_action(VERBOSE_PNR) +) +AlwaysBuild(report_target) + # -- Apio upload. # -- Targets. # -- hardware.bin -> FPGA. -programmer_cmd = get_programmer_cmd(env) +programmer_cmd: str = get_programmer_cmd(env) upload_target = env.Alias("upload", bin_target, programmer_cmd) AlwaysBuild(upload_target) @@ -403,6 +424,7 @@ if GetOption("clean"): env.Default( [ + pnr_report_target, time_target, build_target, verify_out_target, diff --git a/apio/scons/scons_util.py b/apio/scons/scons_util.py index 5fe2d9b8..7b797e19 100644 --- a/apio/scons/scons_util.py +++ b/apio/scons/scons_util.py @@ -19,13 +19,19 @@ import os import re +import json from platform import system from typing import Dict, Tuple, List, Optional from dataclasses import dataclass -import SCons +from SCons import Scanner +from SCons.Node import NodeList +from SCons.Node.FS import File +from SCons.Node.Alias import Alias from SCons.Script import DefaultEnvironment from SCons.Script.SConscript import SConsEnvironment -import SCons.Node.FS + +# import SCons.Node.FS +from SCons.Action import ActionBase, FunctionAction import click # -- Target name @@ -155,24 +161,28 @@ def force_colors(env: SConsEnvironment) -> bool: return flag -def info(env: SConsEnvironment, msg: str) -> None: +def msg(env: SConsEnvironment, text: str, fg: str = None) -> None: + click.secho(text, fg=fg, color=force_colors(env)) + + +def info(env: SConsEnvironment, text: str) -> None: """Prints a short info message and continue.""" - click.secho(f"Info: {msg}") + msg(env, f"Info: {text}") -def warning(env: SConsEnvironment, msg: str) -> None: +def warning(env: SConsEnvironment, text: str) -> None: """Prints a short warning message and continue.""" - click.secho(f"Warning: {msg}", fg="yellow", color=force_colors(env)) + msg(env, f"Warning: {text}", fg="yellow") -def error(env: SConsEnvironment, msg: str) -> None: +def error(env: SConsEnvironment, text: str) -> None: """Prints a short error message and continue.""" - click.secho(f"Error: {msg}", fg="red", color=force_colors(env)) + msg(env, f"Error: {text}", fg="red") -def fatal_error(env: SConsEnvironment, msg: str) -> None: +def fatal_error(env: SConsEnvironment, text: str) -> None: """Prints a short error message and exit with an error code.""" - error(env, msg) + error(env, text) env.Exit(1) @@ -269,7 +279,7 @@ def get_programmer_cmd(env: SConsEnvironment) -> str: return prog_arg -def make_verilog_src_scanner(env: SConsEnvironment) -> SCons.Scanner: +def make_verilog_src_scanner(env: SConsEnvironment) -> Scanner: """Creates and returns a scons Scanner object for scanning verilog files for dependencies. """ @@ -288,7 +298,7 @@ def make_verilog_src_scanner(env: SConsEnvironment) -> SCons.Scanner: ) def verilog_src_scanner_func( - file_node: SCons.Node.FS.File, env: SConsEnvironment, ignored_path + file_node: File, env: SConsEnvironment, ignored_path ) -> List[str]: """Given a Verilog file, scan it and return a list of references to other files it depends on. It's not require to report dependency @@ -342,7 +352,7 @@ def get_source_files(env: SConsEnvironment) -> Tuple[List[str], List[str]]: otherwise as a synthesis file. """ # -- Get a list of all *.v files in the project dir. - files: List[SCons.Node.FS.File] = env.Glob("*.v") + files: List[File] = env.Glob("*.v") # Split file names to synth files and testbench file lists synth_srcs = [] @@ -414,9 +424,9 @@ def get_tests_configs( def make_waves_target( env: SConsEnvironment, - vcd_file_target: SCons.Node.NodeList, + vcd_file_target: NodeList, top_module: str, -) -> List[SCons.Node.Alias.Alias]: +) -> List[Alias]: """Construct a target to launch the QTWave signal viwer. vcd_file_target is the simulator target that generated the vcd file with the signals. Top module is to derive the name of the .gtkw which can be used to save the @@ -521,3 +531,58 @@ def make_verilator_action( ) return action + + +def _print_pnr_report( + env: SConsEnvironment, json_txt: str, verbose: bool +) -> None: + """Accepts the text of the pnr json report and prints it in + a user friendly way. Used by the 'apio report' command.""" + # -- Json text to tree of Dicts. + report: Dict[str, any] = json.loads(json_txt) + + # --- Report utilization + msg(env, "") + msg(env, "DEVICE UTILIZATION:", fg="cyan") + utilization = report["utilization"] + for resource, vals in utilization.items(): + available = vals["available"] + used = vals["used"] + percents = int(100 * used / available) + fg = "magenta" if used > 0 else None + msg( + env, f"{resource:>20}: {used:5} {available:5} {percents:5}%", fg=fg + ) + + # -- Report max clock speed + msg(env, "") + msg(env, "MAX SPEED:", fg="cyan") + speed = report["fmax"] + for clk_net, vals in speed.items(): + clk_signal = clk_net.split("$")[0] + max_mhz = vals["achieved"] + msg(env, f"{clk_signal:>20}: {max_mhz:9.2f} Mhz") + + # -- For now we ignore the critical path report in the pnr report and + # -- refer the user to the pnr verbose output. + msg(env, "") + if not verbose: + msg(env, "For more details use 'apio report --verbose'.", fg="yellow") + + +def get_report_action(verbose: bool) -> ActionBase: + """Returns a SCons action to format and print the PNR reort from the + PNR json report file. Used by the 'apio report' command.""" + + def print_pnr_report( + source: List[File], + target: List[Alias], + env: SConsEnvironment, + ): + """Action function. Loads the pnr json report and print in a user + friendly way.""" + json_file: File = source[0] + json_txt: str = json_file.get_text_contents() + _print_pnr_report(env, json_txt, verbose) + + return FunctionAction(print_pnr_report, {}) diff --git a/test/commands/test_report.py b/test/commands/test_report.py new file mode 100644 index 00000000..578ef7c9 --- /dev/null +++ b/test/commands/test_report.py @@ -0,0 +1,46 @@ +""" + Test for the "apio report" command +""" + +# -- apio report entry point +from apio.commands.report import cli as cmd_report + + +# R0801: Similar lines in 2 files +# pylint: disable=R0801 +def test_report(clirunner, configenv): + """Test: apio report + when no apio.ini file is given + No additional parameters are given + """ + + with clirunner.isolated_filesystem(): + + # -- Config the environment (conftest.configenv()) + configenv() + + # -- Execute "apio report" + result = clirunner.invoke(cmd_report) + + # -- Check the result + assert result.exit_code != 0, result.output + assert "Info: Project has no apio.ini file" in result.output + assert "Error: insufficient arguments: missing board" in result.output + + +def test_report_board(clirunner, configenv): + """Test: apio report + when parameters are given + """ + + with clirunner.isolated_filesystem(): + + # -- Config the environment (conftest.configenv()) + configenv() + + # -- Execute "apio report" + result = clirunner.invoke(cmd_report, ["--board", "icezum"]) + + # -- Check the result + assert result.exit_code != 0, result.output + assert "apio install oss-cad-suite" in result.output diff --git a/test/commands/test_time.py b/test/commands/test_time.py index 9ecbf908..ea48411d 100644 --- a/test/commands/test_time.py +++ b/test/commands/test_time.py @@ -6,6 +6,8 @@ from apio.commands.time import cli as cmd_time +# R0801: Similar lines in 2 files +# pylint: disable=R0801 def test_time(clirunner, configenv): """Test: apio time when no apio.ini file is given From fdb0a88fcf83949638f5d078d94e4a12518e91e3 Mon Sep 17 00:00:00 2001 From: Zapta Date: Sat, 19 Oct 2024 21:10:57 -0700 Subject: [PATCH 05/13] Added implementations of 'apio report' for the ecp5 and gowin architectures. For those architectures, nextpnr generates only utilization report without timing info. --- .gitignore | 5 +++-- apio/scons/ecp5/SConstruct | 32 +++++++++++++++++++++++++++----- apio/scons/gowin/SConstruct | 26 ++++++++++++++++++++++++-- apio/scons/ice40/SConstruct | 2 +- apio/scons/scons_util.py | 18 ++++++++++-------- 5 files changed, 65 insertions(+), 18 deletions(-) diff --git a/.gitignore b/.gitignore index b467dc0e..89fa0429 100644 --- a/.gitignore +++ b/.gitignore @@ -27,10 +27,11 @@ test-colorlight-v61.ice hardware.json hardware.out hardware.vlt -*.out -*.vcd hardware.bit hardware.config +hardware.pnr +*.out +*.vcd .DS_Store abc.history temp-* diff --git a/apio/scons/ecp5/SConstruct b/apio/scons/ecp5/SConstruct index 02e0eef4..fd4f43fd 100644 --- a/apio/scons/ecp5/SConstruct +++ b/apio/scons/ecp5/SConstruct @@ -65,12 +65,13 @@ from apio.scons.scons_util import ( make_waves_target, make_iverilog_action, make_verilator_action, + get_report_action, ) # -- Create the environment env = create_construction_env(ARGUMENTS) -# -- Get arguments +# -- Get arguments. FPGA_SIZE = arg_str(env, "fpga_size", "") FPGA_TYPE = arg_str(env, "fpga_type", "") FPGA_PACK = arg_str(env, "fpga_pack", "") @@ -104,7 +105,7 @@ synth_srcs, test_srcs = get_source_files(env) LPF = get_constraint_file(env, ".lpf", TOP_MODULE) -# -- Apio build/upload. +# -- Apio build/upload/time/report. # -- Builder (yosys, Synthesis). # -- (modules).v -> hardware.json. synth_builder = Builder( @@ -119,16 +120,17 @@ synth_builder = Builder( env.Append(BUILDERS={"Synth": synth_builder}) -# -- Apio build/upload. +# -- Apio build/upload/time/report. # -- builder (nextpnr, Place and route). -# -- hardware.json -> hardware.config. +# -- hardware.json -> hardware.asc, hardware.pnr. pnr_builder = Builder( action=( "nextpnr-ecp5 --{0} --package {1} --json $SOURCE --textcfg $TARGET " - "--lpf {2} {3} --timing-allow-fail --force" + "--report {2}.pnr --lpf {3} {4} --timing-allow-fail --force" ).format( "25k" if (FPGA_TYPE == "12k") else FPGA_TYPE, FPGA_PACK, + TARGET, LPF, "" if VERBOSE_ALL or VERBOSE_PNR else "-q", ), @@ -137,6 +139,16 @@ pnr_builder = Builder( ) env.Append(BUILDERS={"PnR": pnr_builder}) +# -- Apio report. +# -- builder (fake, the .pnr file is generated by pnr_builder) +# -- hardware.config -> hardware.pnr. +pnr_builder = Builder( + action="", + suffix=".pnr", + src_suffix=".config", +) +env.Append(BUILDERS={"PnrReport": pnr_builder}) + # -- Apio build/upload. # -- Builder (icepack, bitstream generator). @@ -167,6 +179,15 @@ if VERBOSE_PNR: if VERBOSE_ALL: AlwaysBuild(synth_target, pnr_target, build_target) +# -- Apio report. +# -- Targets. +# -- hardware.config -> hardware.pnr -> (report) +pnr_report_target = env.PnrReport(TARGET, pnr_target) +report_target = env.Alias( + "report", pnr_report_target, get_report_action(VERBOSE_PNR) +) +AlwaysBuild(report_target) + # -- Apio upload. # -- Targets. @@ -386,6 +407,7 @@ if GetOption("clean"): env.Default( [ + pnr_report_target, build_target, synth_target, pnr_target, diff --git a/apio/scons/gowin/SConstruct b/apio/scons/gowin/SConstruct index 30899f0b..7beb8389 100644 --- a/apio/scons/gowin/SConstruct +++ b/apio/scons/gowin/SConstruct @@ -65,6 +65,7 @@ from apio.scons.scons_util import ( make_waves_target, make_iverilog_action, make_verilator_action, + get_report_action, ) # -- Create the environment @@ -85,6 +86,7 @@ VERILATOR_NO_STYLE = arg_bool(env, "nostyle", False) NOWARNS = arg_str(env, "nowarn", "").split(",") WARNS = arg_str(env, "warn", "").split(",") + # -- Resources paths IVL_PATH = os.environ["IVL"] if "IVL" in os.environ else "" YOSYS_PATH = os.environ["YOSYS_LIB"] if "YOSYS_LIB" in os.environ else "" @@ -101,7 +103,7 @@ synth_srcs, test_srcs = get_source_files(env) # -- Get the CST file name. CST = get_constraint_file(env, ".cst", TOP_MODULE) -# -- Apio build/upload. +# -- Apio build/upload/time/report. # -- Builder (yosys, Synthesis). # -- (modules).v -> hardware.json. synth_builder = Builder( @@ -116,7 +118,7 @@ synth_builder = Builder( env.Append(BUILDERS={"Synth": synth_builder}) -# -- Apio build/upload. +# -- Apio build/upload/time/report. # -- builder (nextpnr, Place and route). # -- hardware.json -> hardware.pnr.json. pnr_builder = Builder( @@ -134,6 +136,16 @@ pnr_builder = Builder( ) env.Append(BUILDERS={"PnR": pnr_builder}) +# -- Apio report. +# -- builder (fake, the .pnr file is generated by pnr_builder) +# -- hardware..pnr.json -> hardware.pnr. +pnr_builder = Builder( + action="", + suffix=".pnr.json", + src_suffix=".asc", +) +env.Append(BUILDERS={"PnrReport": pnr_builder}) + # -- Apio build/upload. # -- Builder (icepack, bitstream generator). @@ -161,6 +173,15 @@ if VERBOSE_PNR: if VERBOSE_ALL: AlwaysBuild(synth_target, pnr_target, build_target) +# -- Apio report. +# -- Targets. +# -- hardware..pnr.json -> hardware.pnr -> (report) +pnr_report_target = env.PnrReport(TARGET, pnr_target) +report_target = env.Alias( + "report", pnr_report_target, get_report_action(VERBOSE_PNR) +) +AlwaysBuild(report_target) + # -- Apio upload. # -- Targets. @@ -377,6 +398,7 @@ if GetOption("clean"): env.Default( [ + pnr_report_target, build_target, verify_out_target, graph_target, diff --git a/apio/scons/ice40/SConstruct b/apio/scons/ice40/SConstruct index 1d09930a..92aa24fa 100644 --- a/apio/scons/ice40/SConstruct +++ b/apio/scons/ice40/SConstruct @@ -191,7 +191,7 @@ if VERBOSE_ALL: # -- Apio report. # -- Targets. -# -- hardware.asd -> hardware.pnr -> (report) +# -- hardware.asc -> hardware.pnr -> (report) pnr_report_target = env.PnrReport(TARGET, pnr_target) report_target = env.Alias( "report", pnr_report_target, get_report_action(VERBOSE_PNR) diff --git a/apio/scons/scons_util.py b/apio/scons/scons_util.py index 7b797e19..b3affda4 100644 --- a/apio/scons/scons_util.py +++ b/apio/scons/scons_util.py @@ -554,14 +554,16 @@ def _print_pnr_report( env, f"{resource:>20}: {used:5} {available:5} {percents:5}%", fg=fg ) - # -- Report max clock speed - msg(env, "") - msg(env, "MAX SPEED:", fg="cyan") - speed = report["fmax"] - for clk_net, vals in speed.items(): - clk_signal = clk_net.split("$")[0] - max_mhz = vals["achieved"] - msg(env, f"{clk_signal:>20}: {max_mhz:9.2f} Mhz") + # -- Report max clock speeds. As of Oct 2024 not available for ECP5 and + # -- Gowin architectures. + clocks = report["fmax"] + if len(clocks) > 0: + msg(env, "") + msg(env, "MAX SPEED:", fg="cyan") + for clk_net, vals in clocks.items(): + clk_signal = clk_net.split("$")[0] + max_mhz = vals["achieved"] + msg(env, f"{clk_signal:>20}: {max_mhz:9.2f} Mhz") # -- For now we ignore the critical path report in the pnr report and # -- refer the user to the pnr verbose output. From 1e208983dca9b36cde1b3ef52e5e6e451349fb46 Mon Sep 17 00:00:00 2001 From: Zapta Date: Sat, 19 Oct 2024 21:49:33 -0700 Subject: [PATCH 06/13] Marked the 'apio time' command as deprecated but it's functionality stayed the same. It has been replaced by the new command 'apio report'. --- apio/commands/build.py | 8 +-- apio/commands/options.py | 118 +++++++++++++++++++++++++-------------- apio/commands/report.py | 11 ++-- apio/commands/time.py | 30 +++------- 4 files changed, 96 insertions(+), 71 deletions(-) diff --git a/apio/commands/build.py b/apio/commands/build.py index cb1d7b6a..c505950f 100644 --- a/apio/commands/build.py +++ b/apio/commands/build.py @@ -47,10 +47,10 @@ @options.verbose_pnr_option @options.top_module_option_gen(deprecated=True) @options.board_option_gen(deprecated=True) -@options.fpga_option -@options.size_option -@options.type_option -@options.pack_option +@options.fpga_option_gen(deprecated=True) +@options.size_option_gen(deprecated=True) +@options.type_option_gen(deprecated=True) +@options.pack_option_gen(deprecated=True) def cli( ctx: Context, # Options diff --git a/apio/commands/options.py b/apio/commands/options.py index 4491cce2..90957589 100644 --- a/apio/commands/options.py +++ b/apio/commands/options.py @@ -107,21 +107,87 @@ def top_module_option_gen( ) +# W0622: Redefining built-in 'help' +# pylint: disable=W0622 +def fpga_option_gen( + *, + deprecated: bool = False, + help: str = "Set the FPGA.", +): + """Generate a --fpga option with given help text.""" + return click.option( + "fpga", # Var name. + "--fpga", + type=str, + metavar="str", + deprecated=deprecated, + help=help, + cls=cmd_util.ApioOption, + ) + + +# W0622: Redefining built-in 'help' +# pylint: disable=W0622 +def size_option_gen( + *, + deprecated: bool = False, + help: str = "Set the FPGA size (1k/8k).", +): + """Generate a --size option with given help text.""" + return click.option( + "size", # Var name + "--size", + type=str, + metavar="str", + deprecated=deprecated, + help=help, + cls=cmd_util.ApioOption, + ) + + +# W0622: Redefining built-in 'help' +# pylint: disable=W0622 +def type_option_gen( + *, + deprecated: bool = False, + help: str = "Set the FPGA type (hx/lp).", +): + """Generate a --type option with given help text.""" + return click.option( + "type_", # Var name. Deconflicting from Python's builtin 'type'. + "--type", + type=str, + metavar="str", + deprecated=deprecated, + help=help, + cls=cmd_util.ApioOption, + ) + + +# W0622: Redefining built-in 'help' +# pylint: disable=W0622 +def pack_option_gen( + *, + deprecated: bool = False, + help: str = "Set the FPGA package.", +): + """Generate a --pack option with given help text.""" + return click.option( + "pack", # Var name + "--pack", + type=str, + metavar="str", + deprecated=deprecated, + help=help, + cls=cmd_util.ApioOption, + ) + + # --------------------------- # -- Static options # --------------------------- -fpga_option = click.option( - "fpga", # Var name. - "--fpga", - type=str, - metavar="str", - deprecated=True, - help="Set the FPGA.", - cls=cmd_util.ApioOption, -) - ftdi_id = click.option( "ftdi_id", # Var name. "--ftdi-id", @@ -130,16 +196,6 @@ def top_module_option_gen( help="Set the FTDI id.", ) -pack_option = click.option( - "pack", # Var name - "--pack", - type=str, - metavar="str", - deprecated=True, - help="Set the FPGA package.", - cls=cmd_util.ApioOption, -) - platform_option = click.option( "platform", # Var name. @@ -190,28 +246,6 @@ def top_module_option_gen( ) -size_option = click.option( - "size", # Var name - "--size", - type=str, - metavar="str", - deprecated=True, - help="Set the FPGA type (1k/8k).", - cls=cmd_util.ApioOption, -) - - -type_option = click.option( - "type_", # Var name. Deconflicting from Python's builtin 'type'. - "--type", - type=str, - metavar="str", - deprecated=True, - help="Set the FPGA type (hx/lp).", - cls=cmd_util.ApioOption, -) - - verbose_option = click.option( "verbose", # Var name. "-v", diff --git a/apio/commands/report.py b/apio/commands/report.py index 861b1953..afaed70f 100644 --- a/apio/commands/report.py +++ b/apio/commands/report.py @@ -29,6 +29,9 @@ Examples: apio report epio report --verbose + +NOTE: The timing reports for the architectures ECP5 and Gowin have not +been implemented yet by the nextpnr project. """ @@ -46,10 +49,10 @@ @options.verbose_option @options.top_module_option_gen(deprecated=True) @options.board_option_gen(deprecated=True) -@options.fpga_option -@options.size_option -@options.type_option -@options.pack_option +@options.fpga_option_gen(deprecated=True) +@options.size_option_gen(deprecated=True) +@options.type_option_gen(deprecated=True) +@options.pack_option_gen(deprecated=True) def cli( ctx: Context, # Options diff --git a/apio/commands/time.py b/apio/commands/time.py index fa6b772e..6c6542b7 100644 --- a/apio/commands/time.py +++ b/apio/commands/time.py @@ -19,20 +19,8 @@ # -- COMMAND # --------------------------- HELP = """ -The time command analyzes -and reports the timing of the design. It is useful to determine the -maximal clock rate that the FPGA can handle with this design. For more -detailed timing information, inspect the file 'hardware.rpt' that the -command generates. -The commands is typically used in the root directory -of the project that contains the apio.ini file. - -\b -Examples: - apio time - -The time command supportw ICE40 devcies. ECP5 and Gowin devices are not -supported yet. +The time command has been deprecated. Please use the 'apio report' command +instead. """ @@ -41,7 +29,7 @@ # pylint: disable=too-many-positional-arguments @click.command( "time", - short_help="Report design timing.", + short_help="[Depreciated] Report design timing.", help=HELP, cls=cmd_util.ApioCommand, ) @@ -50,12 +38,12 @@ @options.verbose_option @options.verbose_yosys_option @options.verbose_pnr_option -@options.top_module_option_gen(deprecated=True) -@options.board_option_gen(deprecated=True) -@options.fpga_option -@options.size_option -@options.type_option -@options.pack_option +@options.top_module_option_gen(deprecated=False) +@options.board_option_gen(deprecated=False) +@options.fpga_option_gen(deprecated=False) +@options.size_option_gen(deprecated=False) +@options.type_option_gen(deprecated=False) +@options.pack_option_gen(deprecated=False) def cli( ctx: Context, # Options From fdcca5aa70c0d684bf39344508723e9cb2ce83bb Mon Sep 17 00:00:00 2001 From: Zapta Date: Sun, 20 Oct 2024 07:42:10 -0700 Subject: [PATCH 07/13] Added a visual studio code debug target for 'apio report'. --- .vscode/launch.json | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.vscode/launch.json b/.vscode/launch.json index 2c0c5c89..5032b83d 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -157,6 +157,18 @@ "justMyCode": false, "cwd": "${workspaceFolder}/test-examples/Alhambra-II/02-jumping-LED" }, + { + "name": "Apio report", + "type": "debugpy", + "request": "launch", + "program": "${workspaceFolder}/apio_run.py", + "args": [ + "report", + ], + "console": "integratedTerminal", + "justMyCode": false, + "cwd": "${workspaceFolder}/test-examples/Alhambra-II/02-jumping-LED" + }, { "name": "Apio system", "type": "debugpy", From 41a3079f15f0e56e1d0adf2504d586d30466e1ee Mon Sep 17 00:00:00 2001 From: Zapta Date: Sun, 20 Oct 2024 08:12:36 -0700 Subject: [PATCH 08/13] Now creating the ActionFunction of 'apio report' in a more standard way. It allows to control the action string printed to the scons logn. --- apio/scons/ecp5/SConstruct | 2 +- apio/scons/gowin/SConstruct | 2 +- apio/scons/ice40/SConstruct | 2 +- apio/scons/scons_util.py | 9 ++++----- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/apio/scons/ecp5/SConstruct b/apio/scons/ecp5/SConstruct index fd4f43fd..60bb1b03 100644 --- a/apio/scons/ecp5/SConstruct +++ b/apio/scons/ecp5/SConstruct @@ -184,7 +184,7 @@ if VERBOSE_ALL: # -- hardware.config -> hardware.pnr -> (report) pnr_report_target = env.PnrReport(TARGET, pnr_target) report_target = env.Alias( - "report", pnr_report_target, get_report_action(VERBOSE_PNR) + "report", pnr_report_target, get_report_action(env, VERBOSE_PNR) ) AlwaysBuild(report_target) diff --git a/apio/scons/gowin/SConstruct b/apio/scons/gowin/SConstruct index 7beb8389..f63bb325 100644 --- a/apio/scons/gowin/SConstruct +++ b/apio/scons/gowin/SConstruct @@ -178,7 +178,7 @@ if VERBOSE_ALL: # -- hardware..pnr.json -> hardware.pnr -> (report) pnr_report_target = env.PnrReport(TARGET, pnr_target) report_target = env.Alias( - "report", pnr_report_target, get_report_action(VERBOSE_PNR) + "report", pnr_report_target, get_report_action(env, VERBOSE_PNR) ) AlwaysBuild(report_target) diff --git a/apio/scons/ice40/SConstruct b/apio/scons/ice40/SConstruct index 92aa24fa..c6b8cf5a 100644 --- a/apio/scons/ice40/SConstruct +++ b/apio/scons/ice40/SConstruct @@ -194,7 +194,7 @@ if VERBOSE_ALL: # -- hardware.asc -> hardware.pnr -> (report) pnr_report_target = env.PnrReport(TARGET, pnr_target) report_target = env.Alias( - "report", pnr_report_target, get_report_action(VERBOSE_PNR) + "report", pnr_report_target, get_report_action(env, VERBOSE_PNR) ) AlwaysBuild(report_target) diff --git a/apio/scons/scons_util.py b/apio/scons/scons_util.py index b3affda4..3012bfb4 100644 --- a/apio/scons/scons_util.py +++ b/apio/scons/scons_util.py @@ -23,16 +23,15 @@ from platform import system from typing import Dict, Tuple, List, Optional from dataclasses import dataclass +import click from SCons import Scanner from SCons.Node import NodeList from SCons.Node.FS import File from SCons.Node.Alias import Alias from SCons.Script import DefaultEnvironment from SCons.Script.SConscript import SConsEnvironment +from SCons.Action import FunctionAction -# import SCons.Node.FS -from SCons.Action import ActionBase, FunctionAction -import click # -- Target name TARGET = "hardware" @@ -572,7 +571,7 @@ def _print_pnr_report( msg(env, "For more details use 'apio report --verbose'.", fg="yellow") -def get_report_action(verbose: bool) -> ActionBase: +def get_report_action(env: SConsEnvironment, verbose: bool) -> FunctionAction: """Returns a SCons action to format and print the PNR reort from the PNR json report file. Used by the 'apio report' command.""" @@ -587,4 +586,4 @@ def print_pnr_report( json_txt: str = json_file.get_text_contents() _print_pnr_report(env, json_txt, verbose) - return FunctionAction(print_pnr_report, {}) + return env.Action(print_pnr_report, "Formatting pnr report.") From ca78d9d2b1b475200a3467d807f6a560daeb6c09 Mon Sep 17 00:00:00 2001 From: Zapta Date: Sun, 20 Oct 2024 09:07:08 -0700 Subject: [PATCH 09/13] Fixed the reported clock name for ecp5 'apio report' command. The net format for ecp5 is different from that of ice40. --- apio/scons/ecp5/SConstruct | 6 ++++-- apio/scons/gowin/SConstruct | 6 ++++-- apio/scons/ice40/SConstruct | 6 ++++-- apio/scons/scons_util.py | 37 +++++++++++++++++++++++++++++++------ 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/apio/scons/ecp5/SConstruct b/apio/scons/ecp5/SConstruct index 60bb1b03..7c3218de 100644 --- a/apio/scons/ecp5/SConstruct +++ b/apio/scons/ecp5/SConstruct @@ -49,6 +49,7 @@ from SCons.Script import ( ) from apio.scons.scons_util import ( TARGET, + SConstructId, is_testbench, basename, is_windows, @@ -183,9 +184,10 @@ if VERBOSE_ALL: # -- Targets. # -- hardware.config -> hardware.pnr -> (report) pnr_report_target = env.PnrReport(TARGET, pnr_target) -report_target = env.Alias( - "report", pnr_report_target, get_report_action(env, VERBOSE_PNR) +report_action = get_report_action( + env, SConstructId.SCONSTRUCT_ECP5, VERBOSE_PNR ) +report_target = env.Alias("report", pnr_report_target, report_action) AlwaysBuild(report_target) diff --git a/apio/scons/gowin/SConstruct b/apio/scons/gowin/SConstruct index f63bb325..b4409774 100644 --- a/apio/scons/gowin/SConstruct +++ b/apio/scons/gowin/SConstruct @@ -49,6 +49,7 @@ from SCons.Script import ( ) from apio.scons.scons_util import ( TARGET, + SConstructId, is_testbench, basename, is_windows, @@ -177,9 +178,10 @@ if VERBOSE_ALL: # -- Targets. # -- hardware..pnr.json -> hardware.pnr -> (report) pnr_report_target = env.PnrReport(TARGET, pnr_target) -report_target = env.Alias( - "report", pnr_report_target, get_report_action(env, VERBOSE_PNR) +report_action = get_report_action( + env, SConstructId.SCONSTRUCT_GOWIN, VERBOSE_PNR ) +report_target = env.Alias("report", pnr_report_target, report_action) AlwaysBuild(report_target) diff --git a/apio/scons/ice40/SConstruct b/apio/scons/ice40/SConstruct index c6b8cf5a..fd73e53e 100644 --- a/apio/scons/ice40/SConstruct +++ b/apio/scons/ice40/SConstruct @@ -49,6 +49,7 @@ from SCons.Script import ( ) from apio.scons.scons_util import ( TARGET, + SConstructId, is_testbench, basename, is_windows, @@ -193,9 +194,10 @@ if VERBOSE_ALL: # -- Targets. # -- hardware.asc -> hardware.pnr -> (report) pnr_report_target = env.PnrReport(TARGET, pnr_target) -report_target = env.Alias( - "report", pnr_report_target, get_report_action(env, VERBOSE_PNR) +report_action = get_report_action( + env, SConstructId.SCONSTRUCT_ICE40, VERBOSE_PNR ) +report_target = env.Alias("report", pnr_report_target, report_action) AlwaysBuild(report_target) diff --git a/apio/scons/scons_util.py b/apio/scons/scons_util.py index 3012bfb4..cce61aa8 100644 --- a/apio/scons/scons_util.py +++ b/apio/scons/scons_util.py @@ -19,6 +19,7 @@ import os import re +from enum import Enum import json from platform import system from typing import Dict, Tuple, List, Optional @@ -33,10 +34,19 @@ from SCons.Action import FunctionAction -# -- Target name +# -- Target name. This is the base file name for various build artifacts. TARGET = "hardware" +class SConstructId(Enum): + """Identifies the SConstruct script that is running. Used to select + the desired behavior when it's script dependent.""" + + SCONSTRUCT_ICE40 = 1 + SCONSTRUCT_ECP5 = 2 + SCONSTRUCT_GOWIN = 3 + + def map_params( env: SConsEnvironment, params: Optional[List[str]], fmt: str ) -> str: @@ -533,7 +543,10 @@ def make_verilator_action( def _print_pnr_report( - env: SConsEnvironment, json_txt: str, verbose: bool + env: SConsEnvironment, + json_txt: str, + script_id: SConstructId, + verbose: bool, ) -> None: """Accepts the text of the pnr json report and prints it in a user friendly way. Used by the 'apio report' command.""" @@ -560,7 +573,15 @@ def _print_pnr_report( msg(env, "") msg(env, "MAX SPEED:", fg="cyan") for clk_net, vals in clocks.items(): - clk_signal = clk_net.split("$")[0] + # TODO: Confirm clk name extraction for Gowin. + # Extract clock name from the net name. + if script_id == SConstructId.SCONSTRUCT_ECP5: + # E.g. '$glbnet$CLK$TRELLIS_IO_IN' -> 'CLK' + clk_signal = clk_net.split("$")[2] + else: + # E.g. 'vclk$SB_IO_IN_$glb_clk' -> 'vclk' + clk_signal = clk_net.split("$")[0] + # Report speed. max_mhz = vals["achieved"] msg(env, f"{clk_signal:>20}: {max_mhz:9.2f} Mhz") @@ -571,9 +592,13 @@ def _print_pnr_report( msg(env, "For more details use 'apio report --verbose'.", fg="yellow") -def get_report_action(env: SConsEnvironment, verbose: bool) -> FunctionAction: +def get_report_action( + env: SConsEnvironment, script_id: SConstructId, verbose: bool +) -> FunctionAction: """Returns a SCons action to format and print the PNR reort from the - PNR json report file. Used by the 'apio report' command.""" + PNR json report file. Used by the 'apio report' command. + 'script_id' identifies the calling SConstruct script and 'verbose' + indicates if the --verbose flag was invoked.""" def print_pnr_report( source: List[File], @@ -584,6 +609,6 @@ def print_pnr_report( friendly way.""" json_file: File = source[0] json_txt: str = json_file.get_text_contents() - _print_pnr_report(env, json_txt, verbose) + _print_pnr_report(env, json_txt, script_id, verbose) return env.Action(print_pnr_report, "Formatting pnr report.") From 6ea8c7409d74e6c89e5dd5a5ef80572516b32434 Mon Sep 17 00:00:00 2001 From: Zapta Date: Sun, 20 Oct 2024 09:21:36 -0700 Subject: [PATCH 10/13] Removed the comment regarding ECP5 not having timing info. Verified with 'apio report' in test-examples/ColorLight-5A-75B-V8/Blinky --- apio/commands/report.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/apio/commands/report.py b/apio/commands/report.py index afaed70f..d96cedb4 100644 --- a/apio/commands/report.py +++ b/apio/commands/report.py @@ -30,8 +30,6 @@ apio report epio report --verbose -NOTE: The timing reports for the architectures ECP5 and Gowin have not -been implemented yet by the nextpnr project. """ From 5e464b466cf6a1b70ea5b294343462c27e345ffe Mon Sep 17 00:00:00 2001 From: Zapta Date: Sun, 20 Oct 2024 09:39:58 -0700 Subject: [PATCH 11/13] Updated a comment regarding missing timing information. No code change. --- apio/scons/scons_util.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apio/scons/scons_util.py b/apio/scons/scons_util.py index cce61aa8..8938dc31 100644 --- a/apio/scons/scons_util.py +++ b/apio/scons/scons_util.py @@ -566,8 +566,11 @@ def _print_pnr_report( env, f"{resource:>20}: {used:5} {available:5} {percents:5}%", fg=fg ) - # -- Report max clock speeds. As of Oct 2024 not available for ECP5 and - # -- Gowin architectures. + # -- Report max clock speeds. + # -- + # -- NOTE: As of Oct 2024, some projects do not generate timing + # -- information and this is being investigated. + # -- See https://github.com/FPGAwars/icestudio/issues/774 for details. clocks = report["fmax"] if len(clocks) > 0: msg(env, "") From b4bf6e7f46fc82ae83e32707b8a8d36fc8f8ce6e Mon Sep 17 00:00:00 2001 From: Zapta Date: Sun, 20 Oct 2024 10:27:41 -0700 Subject: [PATCH 12/13] In 'apio -h' help text, splitted the 'project' group into 'build' and 'verification' groups. The goal is to simplify the initial user experience (a small build group) and to emphasize the veriication aspect to more experience users. No change in command behavior. Also, simplified the grouping code. --- apio/__main__.py | 100 ++++++++++++++++++++++------------------ apio/commands/clean.py | 2 +- apio/commands/verify.py | 2 +- 3 files changed, 56 insertions(+), 48 deletions(-) diff --git a/apio/__main__.py b/apio/__main__.py index 500fb84a..1efd5e6a 100644 --- a/apio/__main__.py +++ b/apio/__main__.py @@ -17,6 +17,40 @@ import click from apio import util +# -- Maps group title to command names. Controls how the 'apio -h' help +# -- information is printed. Should include all commands and without +# -- duplicates. +COMMAND_GROUPS = { + "Build commands": [ + "build", + "upload", + "clean", + ], + "Verification commands": [ + "verify", + "lint", + "sim", + "test", + "time", + "report", + "graph", + ], + "Setup commands": [ + "create", + "modify", + "drivers", + "install", + "uninstall", + ], + "Utility commands": [ + "boards", + "examples", + "raw", + "system", + "upgrade", + ], +} + def select_commands_help( command_lines: List[str], command_names: List[str] @@ -70,56 +104,30 @@ def reformat_apio_help(original_help: str) -> str: index += 1 # Skip the Commands: line. command_lines = help_lines[index:] - # -- Select project commands by the order they are listed here. - project_help = select_commands_help( - command_lines, - [ - "build", - "clean", - "verify", - "sim", - "test", - "lint", - "upload", - "time", - "report", - "graph", - ], - ) - # -- Select setup commands by the order they are listed here. - setup_help = select_commands_help( - command_lines, - ["create", "modify", "drivers", "install", "uninstall"], - ) - - # -- Select utility commands by the order they are listed here. - utility_help = select_commands_help( - command_lines, - ["boards", "examples", "raw", "system", "upgrade"], - ) - - # -- Sanity check, in case we mispelled or ommited a command name. - num_selected = len(project_help) + len(setup_help) + len(utility_help) - assert len(command_lines) == num_selected - # -- Header result = [] result.extend(header_lines) - # -- Project commands: - result.append("Project commands:") - result.extend(project_help) - result.append("") - - # -- Setup commands: - result.append("Setup commands:") - result.extend(setup_help) - result.append("") - - # -- Print utility commands: - result.append("Utility commands:") - result.extend(utility_help) - result.append("") + # -- Print the help of the command groups while verifying that there + # -- are no missing or duplicate commands. + reported_commands = set() + for group_title, command_names in COMMAND_GROUPS.items(): + # -- Assert no duplicates inter and intra groups. + assert len(command_names) == len(set(command_names)) + assert reported_commands.isdisjoint(command_names) + reported_commands.update(command_names) + # -- Select the help lines of the commands in this group. + group_help = select_commands_help(command_lines, command_names) + # -- Append the group title and command lines. + result.append(f"{group_title}:") + result.extend(group_help) + result.append("") + + # -- If this assertion fails, for a missing command in the groups + # -- definitions. + assert len(command_lines) == len( + reported_commands + ), f"{command_lines=}, {len(reported_commands)=}" return "\n".join(result) diff --git a/apio/commands/clean.py b/apio/commands/clean.py index b6145283..2e66ec5a 100644 --- a/apio/commands/clean.py +++ b/apio/commands/clean.py @@ -35,7 +35,7 @@ @click.command( "clean", - short_help="Clean the apio generated files.", + short_help="Delete the apio generated files.", help=HELP, cls=cmd_util.ApioCommand, ) diff --git a/apio/commands/verify.py b/apio/commands/verify.py index 3af1e9d5..f164dbdf 100644 --- a/apio/commands/verify.py +++ b/apio/commands/verify.py @@ -36,7 +36,7 @@ @click.command( "verify", - short_help="Verify project's verilog code.", + short_help="Verify the verilog code.", help=HELP, cls=cmd_util.ApioCommand, ) From cb1950a8325a5227489d2fac42943042c30f3795 Mon Sep 17 00:00:00 2001 From: Zapta Date: Sun, 20 Oct 2024 13:07:26 -0700 Subject: [PATCH 13/13] Minor tweaks of the 'apio report' output format. --- apio/scons/scons_util.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/apio/scons/scons_util.py b/apio/scons/scons_util.py index 8938dc31..a05ee562 100644 --- a/apio/scons/scons_util.py +++ b/apio/scons/scons_util.py @@ -555,7 +555,7 @@ def _print_pnr_report( # --- Report utilization msg(env, "") - msg(env, "DEVICE UTILIZATION:", fg="cyan") + msg(env, "UTILIZATION:", fg="cyan") utilization = report["utilization"] for resource, vals in utilization.items(): available = vals["available"] @@ -571,10 +571,10 @@ def _print_pnr_report( # -- NOTE: As of Oct 2024, some projects do not generate timing # -- information and this is being investigated. # -- See https://github.com/FPGAwars/icestudio/issues/774 for details. + msg(env, "") + msg(env, "CLOCKS:", fg="cyan") clocks = report["fmax"] if len(clocks) > 0: - msg(env, "") - msg(env, "MAX SPEED:", fg="cyan") for clk_net, vals in clocks.items(): # TODO: Confirm clk name extraction for Gowin. # Extract clock name from the net name. @@ -586,13 +586,14 @@ def _print_pnr_report( clk_signal = clk_net.split("$")[0] # Report speed. max_mhz = vals["achieved"] - msg(env, f"{clk_signal:>20}: {max_mhz:9.2f} Mhz") + styled_max_mhz = click.style(f"{max_mhz:7.2f}", fg="magenta") + msg(env, f"{clk_signal:>20}: {styled_max_mhz} Mhz max") # -- For now we ignore the critical path report in the pnr report and # -- refer the user to the pnr verbose output. msg(env, "") if not verbose: - msg(env, "For more details use 'apio report --verbose'.", fg="yellow") + msg(env, "Use 'apio report --verbose' for more details.", fg="yellow") def get_report_action(