Fix clock target - #169
Open
quetric wants to merge 5 commits into
Open
Conversation
3 tasks
Realign the user-clock work with dev, which advanced 70 commits since the
merge base (compute-only shell split, HSA graph API, LSF enablement, vrtd
shell-state/reset).
Only two conflicts, both import blocks:
* linker/slashkit/__main__.py -- keep dev's parenthesised command_config
import (adds ShellType) alongside the apply_timing_frequency_cap import.
* linker/slashkit/emit/hw/project_gen.py -- keep dev's command_config and
core.launcher TASK_* imports, and extend the timing_freq import with
read_system_map_clock_hz.
Everything else auto-merged. Verified in the merged tree:
* base/scripts/slash_project_build.tcl moved to base/common/scripts/ in dev;
rename detection carried the --user-clock-xdc handling across, so the
constraint now applies to both the service and compute shells.
* _generate_user_clock_xdc() landed in the RM_KIND.SLASH_PROJECT branch of
dev's restructured _run_rm_build.
* RM_KIND.SLASH_PROJECT = "slash" gives build_dir/slash_rm, matching the
hardcoded report_timing_<proj>.txt path the tcl writes to.
* CLOCK_MAX_FREQ removed with no dangling references.
* clock.c: dev's BAR4->BAR2 fallback and the no_exceed parameter are
disjoint; serve.c is the only caller and is updated.
* wire.h: VRTD_CLOCK_OP_SET_ROUND_DOWN is a new value in an existing op
field, so it composes with dev's new opcode and widened structs.
* smi clockwiz.cpp still binds setClockRate via the roundDown default.
linker: 162 tests pass, including the three new timing_freq tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The user-clock constraint chain was only half connected. --clock-hz produced a scoped create_clock for the implementation run, but the RM block design still declared user_clk at the 200 MHz hardcoded in slash_base.bd. Every AXI IP inside the reconfigurable module was therefore configured, and the out-of-context constraints its synthesis run is timed against were derived, for 200 MHz no matter what was asked for. Pass the resolved target through as --user-clock-hz and apply it with set_property CONFIG.FREQ_HZ on the BD's user_clk port before the kernels are added, so the generated BD Tcl's trailing validate_bd_design propagates it downstream. Only user_clk moves; the AXI interfaces crossing the partition boundary run on static_region_clk at 400 MHz and are unaffected. Both halves now come from the same resolved target, read back from system_map.xml so there is a single source of truth. The path is in base/common, so it applies identically to the service and compute shells. Verified on both shells with --clock-hz 250000000: slash_base_ooc.xdc carries "create_clock -name user_clk -period 4" (was 5) with static_region_clk unchanged at 2.500, and the implementation clock summary reports user_clk at 4.000 ns / 250 MHz. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
--clock-hz is the frequency the reconfigurable module is implemented against, not a ceiling to aim at, and overshooting it costs performance rather than gaining it. An over-constrained implementation routes worse than an achievable one, and the recorded frequency is derated from that worse result. Measured on 00_axilite, asking for 250 MHz pushed the critical path out by ~1.2-1.3 ns on both shells relative to asking for 200 MHz: service ask 200 -> 184 MHz delivered ask 250 -> 146 MHz delivered compute ask 200 -> 200 MHz (met) ask 250 -> 161 MHz delivered Asking for more produced a slower device on both. That is unintuitive enough to deserve more than a log line, so print a warning naming the requested and recorded frequencies and pointing at re-linking lower. The warning stays quiet when the target is met, so it does not become noise. Also record the known two-clock limitation next to the constraint that causes it. The scoped create_clock leaves user_clk and the shell's clkout1_primitive_2 on one physical net, and Vivado times the few static-to-RM crossings against the beat frequency of the two periods (a bogus 1 ns requirement at 250 MHz) rather than treating them as the same clock. Costs under 1 MHz on both shells, and is latent at the default 200 MHz where the two clocks share a 5 ns grid. The same comment previously claimed the RM port was the only load on that net; the timing report shows otherwise, so that claim is corrected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
quetric
marked this pull request as ready for review
August 15, 2026 22:07
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #98