Skip to content

feat: integrate ecc-sizer timing opt tool#110

Open
zhaoxueyan1 wants to merge 3 commits into
mainfrom
integrate-sizer
Open

feat: integrate ecc-sizer timing opt tool#110
zhaoxueyan1 wants to merge 3 commits into
mainfrom
integrate-sizer

Conversation

@zhaoxueyan1

@zhaoxueyan1 zhaoxueyan1 commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary

Scope

Select the areas touched by this PR:

  • CLI - command behavior, Typer command surface, output formats, or workspace commands.
  • Flow/runtime - workspace lifecycle, EngineFlow, step execution, logs, metrics, or artifacts.
  • EDA integration - Yosys, ECC-Tools, DreamPlace, KLayout, PDKs, or native/runtime wrappers.
  • Build/package - Bazel, Nix, PyInstaller, wheels, uv.lock, or release artifacts.
  • CI/release - GitHub Actions, version checks, changelog, or release automation.
  • Tests/docs only

What Changed

Validation

List the commands you ran. Mark checks that are not applicable as N/A.

  • uv run pytest test/
  • Focused pytest:
  • uv run ruff check chipcompiler test
  • uv run ruff format --check chipcompiler test
  • bazel build //:build_ecc_cli_bundle
  • PyInstaller smoke: ecc --help, ecc --version, ecc version --json
  • Nix smoke: nix run .#cli -- --help
  • Manual flow smoke:
  • Other:

Skipped checks and reason:

Runtime And Packaging Impact

  • No runtime or packaging impact
  • CLI output or machine-readable contract changed
  • Workspace layout, flow state, or artifact paths changed
  • Native toolchain or wrapper behavior changed
  • ecc-tools or ecc-dreamplace dependency changed
  • PyInstaller, Nix, Bazel, or release artifact changed

Notes:

Checklist

  • I kept the change scoped to ECC.
  • I updated docs or user-facing CLI text where behavior changed.
  • I included lockfile or version metadata updates when dependencies changed.
  • I documented any submodule updates and why they are needed.
  • I did not include local caches, virtual environments, or generated build outputs.
  • I explained skipped validation and remaining risk.

Copilot AI review requested due to automatic review settings June 8, 2026 12:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Integrates the ECC-Sizer “Sizer” binary into the ECC toolchain so the flow can run timing optimization steps via the new ecc_sizer builder/runner, with Bazel/dev-environment support and tests.

Changes:

  • Add chipcompiler.tools.ecc_sizer (builder/runner/utility) and hook it into tool-module resolution and workspace CLI.
  • Extend flow step result validation so TIMING_OPT steps succeed with DEF+Verilog outputs (no GDS required).
  • Add Bazel CMake build + install scripts and update dev docs/tests for ECC-Sizer.

Reviewed changes

Copilot reviewed 19 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/test_ecc_sizer_module.py Adds unit tests for sizer step config, runner invocation, and TIMING_OPT result validation.
MODULE.bazel.lock Updates Bazel module lockfile registry hash URLs (now pointing at a mirror).
MODULE.bazel Adds local source repo for ecc-sizer thirdparty source tree.
docs/development.md Documents ECC-Sizer dev build/install and submodule init requirements.
chipcompiler/tools/eda.py Adds sizerecc_sizer alias for module lookup.
chipcompiler/tools/ecc/builder.py Adds a StepEnum.TIMING_OPT data directory mapping.
chipcompiler/tools/ecc_sizer/utility.py Adds helpers to locate sizer source root and runtime binary, with env overrides.
chipcompiler/tools/ecc_sizer/runner.py Adds runner that executes Sizer with generated env/cmd files and validates outputs.
chipcompiler/tools/ecc_sizer/builder.py Adds builder that generates sizer env/cmd files based on workspace/PDK parameters.
chipcompiler/tools/ecc_sizer/init.py Exposes ecc_sizer builder/runner/utility API.
chipcompiler/thirdparty/BUILD.bazel Adds ecc_sizer_cmake Bazel CMake build rule to build the Sizer binary.
chipcompiler/pyinstaller_utils.py Excludes ecc-sizer thirdparty subtree from PyInstaller payload.
chipcompiler/engine/flow.py Treats TIMING_OPT variants as successful when DEF+Verilog exist (no GDS).
chipcompiler/cli/workspace/service.py Adds sizerecc_sizer alias for CLI builder imports.
chipcompiler/BUILD.bazel Excludes thirdparty/ecc-sizer/** from Python globs.
bazel/scripts/prepare-dev.sh Runs install-sizer during prepare_dev and prints guidance on failure.
bazel/scripts/install-sizer.sh Adds script to install/clean Bazel-built Sizer into the source tree for dev use.
bazel/scripts/BUILD.bazel Wires install_sizer/clean_sizer and adds sizer CMake artifact to prepare-dev.
.gitmodules Adds ecc-sizer submodule entry.
.gitignore Ignores installed chipcompiler/tools/ecc_sizer/bin/ artifacts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +45 to +46
env_text = open(step.script["sizer_env"], encoding="utf-8").read()
cmd_text = open(step.script["sizer_cmd"], encoding="utf-8").read()
Comment on lines +84 to +86
os.makedirs(os.path.dirname(step.output["def"]), exist_ok=True)
open(step.output["def"], "w", encoding="utf-8").write("def\n")
open(step.output["verilog"], "w", encoding="utf-8").write("module gcd; endmodule\n")
Comment thread .gitmodules Outdated
Comment thread chipcompiler/thirdparty/BUILD.bazel
@Emin017 Emin017 self-requested a review June 8, 2026 12:22

@Emin017 Emin017 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have made some comments on the construction, and I will test the flow process locally.

Please also add timing optimization step in rtl2gds flow, so we can test this flow on ci (test/test_tools.py).

def build_rtl2gds_flow() -> list:

Comment thread chipcompiler/thirdparty/BUILD.bazel Outdated
"BUILD_GUI": "OFF",
"BUILD_PYTHON": "OFF",
"CMAKE_BUILD_TYPE": "Release",
"CMAKE_CXX_COMPILER": "/usr/bin/g++",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do not hardcode this.

Comment thread .gitmodules Outdated
set -euo pipefail

WS="${BUILD_WORKSPACE_DIRECTORY:?Must run via: bazel run //bazel/scripts:install_sizer}"
SIZER_RUNTIME_DIR="${WS}/chipcompiler/tools/ecc_sizer/bin"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is sizer added to chipcompiler/tools/ecc_sizer/bin as an executable file? I think injecting the executable file path into get_sizer_command is a better way to handle it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Due to the need to distribute ecc cli, it may be necessary to consider building sizer as an independent python wheel or ecc monorepo.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

CC @Anillc

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be better to build outside the tree and provide the executable by PATH like yosys.

Comment thread chipcompiler/thirdparty/BUILD.bazel
@Emin017 Emin017 added the enhancement New feature or request label Jun 9, 2026
@Emin017 Emin017 added this to the 0.1.0-Alpha milestone Jun 9, 2026
@Emin017 Emin017 changed the title Integrate ecc-sizer timing opt tool feat: integrate ecc-sizer timing opt tool Jun 9, 2026
zhaoxueyan1 and others added 2 commits June 9, 2026 12:12
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants