From ea386172fed05c26a50670bd4040a3a6a5993bd8 Mon Sep 17 00:00:00 2001 From: Harry Callahan Date: Sun, 25 Oct 2026 02:37:31 +0000 Subject: [PATCH] feat: fall back to a built mode when the primary build mode isn't built MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A sim cfg's primary_build_mode says whose coverage goes first in the merge. If a run selects only tests that use other build modes, and more than one of them, dvsim stops with "primary_build_mode ... is invalid", even though nothing else in the run depends on that build. So a cfg whose default build is not the primary one, or a run of only the tests that need special builds, can't be run without editing the cfg. Use the first of the run's builds by name instead, so the choice is the same from run to run, and warn if coverage is enabled (without it, nothing reads the primary build mode). A primary_build_mode that names no build mode of the cfg is still an error. A run that does build the primary mode is unchanged. AI-assisted (Claude Code) — reviewed and approved by author Co-Authored-By: Claude Signed-off-by: Harry Callahan --- doc/design_doc.md | 4 ++ src/dvsim/sim/flow.py | 33 ++++++--- tests/sim/__init__.py | 5 ++ tests/sim/test_build_modes.py | 132 ++++++++++++++++++++++++++++++++++ 4 files changed, 166 insertions(+), 8 deletions(-) create mode 100644 tests/sim/__init__.py create mode 100644 tests/sim/test_build_modes.py diff --git a/doc/design_doc.md b/doc/design_doc.md index 7865918c..42bfbbae 100644 --- a/doc/design_doc.md +++ b/doc/design_doc.md @@ -187,6 +187,10 @@ If coverage is enabled, it creates additional deployable objects for merging the The deployable objects representing the tests are added as dependencies of the coverage object, so that the coverage merge job is run only after all tests finish. Thus, all [jobs](#eda-tool-flow-steps) have an associated deployable object, which manages it. +The coverage merge puts the coverage of the cfg's `primary_build_mode` first. +If no test of the run uses that build mode, dvsim uses the first of the run's builds by name instead (and warns, if coverage is enabled). +`primary_build_mode` must still name a build mode of the cfg. + ### Scheduler stage Once a dependency-encoded list of deployable objects is constructed, the entire list is passed onto the **scheduler**. diff --git a/src/dvsim/sim/flow.py b/src/dvsim/sim/flow.py index a907a832..4034bb16 100644 --- a/src/dvsim/sim/flow.py +++ b/src/dvsim/sim/flow.py @@ -243,9 +243,9 @@ def _expand(self) -> None: # Set the primary build mode. The coverage associated to this build # is the main coverage. Some tools need this information. This is - # of significance only when there are multiple builds. If there is - # only one build, and its not the primary_build_mode, then we - # update the primary_build_mode to match what is built. + # of significance only when there are multiple builds. If it is not + # among the builds of a run, _create_deploy_objects picks one that + # is (the only one, or the first by name). if not hasattr(self, "primary_build_mode"): self.primary_build_mode = self.build_mode @@ -540,12 +540,29 @@ def _create_deploy_objects(self) -> None: log.error("Nothing to do as no matching jobs could be found.") sys.exit(1) if self.primary_build_mode not in build_mode_names: - log.error( - f'"primary_build_mode: {self.primary_build_mode}" ' - f"in {self.name} cfg is invalid. Please pick from " - f"{build_mode_names}.", + if self.primary_build_mode not in {m.name for m in self.build_modes}: + log.error( + f'"primary_build_mode: {self.primary_build_mode}" ' + f"in {self.name} cfg is not a build mode. Please pick from " + f"{sorted(m.name for m in self.build_modes)}.", + ) + sys.exit(1) + + # The primary build mode only says whose coverage goes first in + # the merge, so a run whose tests all use other build modes can + # still go ahead: pick one of those, the same one every time. + # Without coverage nothing reads it, so there is nothing to warn about. + fallback = min(build_mode_names) + log.log( + log.WARNING if self.cov else log.VERBOSE, + '"primary_build_mode: %s" in %s cfg is not among the builds ' + "of this run (%s). Using %s instead.", + self.primary_build_mode, + self.name, + ", ".join(sorted(build_mode_names)), + fallback, ) - sys.exit(1) + self.primary_build_mode = fallback # Update all tests to use the updated (uniquified) build modes. for test in self.run_list: diff --git a/tests/sim/__init__.py b/tests/sim/__init__.py new file mode 100644 index 00000000..0a8dd63d --- /dev/null +++ b/tests/sim/__init__.py @@ -0,0 +1,5 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""DVSim sim flow tests.""" diff --git a/tests/sim/test_build_modes.py b/tests/sim/test_build_modes.py new file mode 100644 index 00000000..7a3e0950 --- /dev/null +++ b/tests/sim/test_build_modes.py @@ -0,0 +1,132 @@ +# Copyright lowRISC contributors (OpenTitan project). +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for how SimCfg picks the primary build mode of a run.""" + +import logging +from dataclasses import dataclass, field + +import pytest + +from dvsim.sim import flow +from dvsim.sim.flow import SimCfg + +__all__ = () + + +@dataclass(frozen=True) +class FakeBuildMode: + """A build mode; builds of two modes with the same key are equivalent.""" + + name: str + key: str = field(default="", compare=False) + + +class FakeCompileSim: + """Stands in for CompileSim: a build named after its build mode.""" + + def __init__(self, build_mode_obj: FakeBuildMode, _sim_cfg: SimCfg) -> None: + """Make the build of a build mode.""" + self.name = build_mode_obj.name + self.key = build_mode_obj.key or build_mode_obj.name + + def is_equivalent_job(self, other: "FakeCompileSim") -> bool: + """Whether two builds are interchangeable.""" + return self.key == other.key + + +def _deploy( + monkeypatch: pytest.MonkeyPatch, + primary: str, + *modes: FakeBuildMode, + defined: tuple[str, ...] = (), + cov: bool = False, +) -> SimCfg: + """Run _create_deploy_objects for a build-only run of the given modes. + + The SimCfg is made without its constructor, which needs a whole project; only the + attributes that _create_deploy_objects reads are set. defined names build modes of the + cfg that the run doesn't build. + """ + monkeypatch.setattr(flow, "CompileSim", FakeCompileSim) + cfg = object.__new__(SimCfg) + cfg.__dict__.update( + name="blk", + tool="vcs", + primary_build_mode=primary, + build_list=list(modes), + run_list=[], + build_modes=[*modes, *map(FakeBuildMode, defined)], + build_only=True, + run_only=False, + gui=False, + gui_debug=False, + cov=cov, + ) + cfg._create_build_and_run_list = lambda: None + cfg._create_deploy_objects() + return cfg + + +class TestPrimaryBuildMode: + """Picking the primary build mode of a run.""" + + def test_kept_when_built(self, monkeypatch: pytest.MonkeyPatch) -> None: + cfg = _deploy(monkeypatch, "default", FakeBuildMode("other"), FakeBuildMode("default")) + assert cfg.primary_build_mode == "default" + + def test_single_build_becomes_primary(self, monkeypatch: pytest.MonkeyPatch) -> None: + cfg = _deploy(monkeypatch, "default", FakeBuildMode("other")) + assert cfg.primary_build_mode == "other" + + def test_follows_an_equivalent_build(self, monkeypatch: pytest.MonkeyPatch) -> None: + cfg = _deploy( + monkeypatch, + "cover", + FakeBuildMode("plain", key="same"), + FakeBuildMode("cover", key="same"), + FakeBuildMode("other"), + ) + assert cfg.primary_build_mode == "plain" + + def test_falls_back_when_not_built( + self, monkeypatch: pytest.MonkeyPatch, dvsim_log: pytest.LogCaptureFixture + ) -> None: + cfg = _deploy( + monkeypatch, + "default", + FakeBuildMode("zeta"), + FakeBuildMode("alpha"), + defined=("default",), + cov=True, + ) + + assert cfg.primary_build_mode == "alpha" + assert [b.name for b in cfg.builds] == ["zeta", "alpha"] + assert any( + '"primary_build_mode: default"' in r.getMessage() and "Using alpha" in r.getMessage() + for r in dvsim_log.records + ) + + def test_fallback_without_coverage_does_not_warn( + self, monkeypatch: pytest.MonkeyPatch, dvsim_log: pytest.LogCaptureFixture + ) -> None: + cfg = _deploy( + monkeypatch, + "default", + FakeBuildMode("zeta"), + FakeBuildMode("alpha"), + defined=("default",), + ) + assert cfg.primary_build_mode == "alpha" + assert not [r for r in dvsim_log.records if r.levelno >= logging.WARNING] + + def test_undefined_primary_is_an_error(self, monkeypatch: pytest.MonkeyPatch) -> None: + """A primary_build_mode that names no build mode is a typo, not a fallback.""" + with pytest.raises(SystemExit): + _deploy(monkeypatch, "defualt", FakeBuildMode("zeta"), FakeBuildMode("alpha")) + + def test_nothing_to_do_still_fails(self, monkeypatch: pytest.MonkeyPatch) -> None: + with pytest.raises(SystemExit): + _deploy(monkeypatch, "default")