Skip to content

artix7, kintex7, spartan7, zynq7: DSP48E1 SEL_PATTERN_C - #33

Merged
cavearr merged 1 commit into
openXC7:masterfrom
HeroesLament:dsp48e1-selpat-c
Oct 14, 2026
Merged

cavearr merged 1 commit into
openXC7:masterfrom
HeroesLament:dsp48e1-selpat-c

Conversation

@HeroesLament

@HeroesLament HeroesLament commented Oct 11, 2026 •

Copy link
Copy Markdown

SEL_PATTERN=C had no segbits row, so nextpnr could not distinguish
a design comparing against the C port from one comparing against the
PATTERN attribute.

The bit was found outside the fuzzer: probe designs selecting each
SEL_PATTERN value, diffed against each other, then measured on
xc7z020 silicon across three boards. Dumps were byte-identical
across boards. SEL_PATTERN=C is bit 26_80 alone (DSP_0; 26_240 in
DSP_1), with SEL_MASK_C/ROUNDING_MODE clear.

SEL_PATTERN=A shows no dedicated bit in the same probes, so no row
is added for it.

mask_dsp_[lr] gets 26_80/26_240 as well; a fuzzer run would add them
naturally, a hand-landed row has to state them.

The fuzzer companion is openXC7/prjxray#31, a variant of
100-dsp-mskpat that fuzzes SEL_PATTERN, so the rows regenerate from
a corpus.

Rows are identical in the four families. virtex7 is not changed:
db-diffed only, not measured (same as 82cb7d8).

@sourcery-ai

sourcery-ai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds the missing DSP48E1 segbits and origin metadata for SEL_PATTERN=C in Artix-7, Kintex-7, Spartan-7, and Zynq-7000 databases, using the measured standalone pattern-detect bit while intentionally making no Virtex-7 or SEL_PATTERN=A change.

File-Level Changes

Change Details Files
Add DSP48E1 pattern-detect encoding for SEL_PATTERN=C across the supported 7-series families.
  • Map SEL_PATTERN=C to the standalone pattern-detect bit for left and right DSP database variants.
  • Record matching provenance metadata for each new segbits entry.
  • Keep SEL_MASK_C and ROUNDING_MODE out of the encoding; leave SEL_PATTERN=A unchanged because probes showed no dedicated bit.
artix7/segbits_dsp_l.db
artix7/segbits_dsp_l.origin_info.db
artix7/segbits_dsp_r.db
artix7/segbits_dsp_r.origin_info.db
kintex7/segbits_dsp_l.db
kintex7/segbits_dsp_l.origin_info.db
kintex7/segbits_dsp_r.db
kintex7/segbits_dsp_r.origin_info.db
spartan7/segbits_dsp_l.db
spartan7/segbits_dsp_l.origin_info.db
spartan7/segbits_dsp_r.db
spartan7/segbits_dsp_r.origin_info.db
zynq7/segbits_dsp_l.db
zynq7/segbits_dsp_l.origin_info.db
zynq7/segbits_dsp_r.db
zynq7/segbits_dsp_r.origin_info.db

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot 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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

@hansfbaier

Copy link
Copy Markdown

First - thank you for the contribution. In order to keep this maintainable we have made it a rule to also need the commit to prjxray which generates these changes. If you generated this in another way, please let us know.

@HeroesLament

Copy link
Copy Markdown
Author

Thanks @hansfbaier, that would be a lot easier probably! I applied these directly to the xilinx db definitions, but I will bring these changes over to the prjxray repo as well. I'll have that up within the hour.

I appreciate the fast reply on the weekend, thank you :)

Add the SEL_PATTERN=C bit found by the dsp_char probe campaign, and its
mask_dsp_[lr] entries, which a fuzzer run would add naturally but a
hand-landed row has to state explicitly.
HeroesLament pushed a commit to HeroesLament/prjxray that referenced this pull request Oct 11, 2026
New fuzzer variant of 100-dsp-mskpat. SEL_PATTERN was never fuzzed,
so the corpus only ever saw Vivado's default (PATTERN) and
SEL_PATTERN_C could never solve. top.py fuzzes it over its full enum
{PATTERN, C}; generate.py emits the tag group through
add_site_group_zero inside the USE_PATTERN_DETECT=PATDET guard,
mirroring SEL_MASK. PATTERN is the zero value; only C carries a bit.

Companion: openXC7/prjxray-db#33.
HeroesLament pushed a commit to HeroesLament/prjxray that referenced this pull request Oct 11, 2026
Variant of 100-dsp-mskpat. SEL_PATTERN was never fuzzed, so
specimens only ever used Vivado's default (PATTERN) and the
SEL_PATTERN_C bit could never solve. top.py fuzzes SEL_PATTERN over
{PATTERN, C}; generate.py tags it with add_site_group_zero, same as
SEL_MASK. PATTERN is the zero value, only C has a bit.

Companion: openXC7/prjxray-db#33.
@HeroesLament

Copy link
Copy Markdown
Author

Hi @hansfbaier I have implemented the proper prjxray fuzzer tooling to generate the DB changes in the openXC7/prjxray repo!

openXC7/prjxray#31

This should hopefully be a better implementation for DSP cell users going forward.

HeroesLament pushed a commit to HeroesLament/prjxray that referenced this pull request Oct 11, 2026
Variant of 100-dsp-mskpat. SEL_PATTERN was never fuzzed, so
specimens only ever used Vivado's default (PATTERN) and the
SEL_PATTERN_C bit could never solve. top.py fuzzes SEL_PATTERN over
{PATTERN, C}; generate.py tags it with add_site_group_zero, same as
SEL_MASK. PATTERN is the zero value, only C has a bit.

Companion: openXC7/prjxray-db#33.
@cavearr

cavearr commented Oct 12, 2026

Copy link
Copy Markdown

I diffed d3405db against 75825f2 row by row.

Every family except virtex7 gains the same two features, on DSP_L and on DSP_R:

DSP_0.SEL_PATTERN_C 26_80
DSP_1.SEL_PATTERN_C 26_240

No segbit at 75825f2 already used 26_80 or 26_240. The mask files of artix7, spartan7 and zynq7 each gain bit 26_80 and bit 26_240 and keep the previous order. The segbit order is kept in every family.

kintex7/mask_dsp_l.db is +340/−338 and kintex7/mask_dsp_r.db is +448/−446. The set of lines grows by those same two bits and loses nothing (1628 to 1630, and 1941 to 1943). The shared lines were re-sorted. The base file is a string sort, so bit 00_106 comes before bit 00_42. The new file is sorted by frame and bit number. I would put the base order back and append the two bits, so the diff is +2 like the other families. The reorder is not new data from the silicon.

virtex7 is byte-identical: mask_l 3095, mask_r 2951, segbits 436. That matches leaving it untouched. I have not measured these addresses on a Virtex-7 part. Its segbits file was already four lines shorter than the others before this change.

The addresses match the Vivado golden in the note on #95, and a reduced run of prjxray#31.

@cavearr

cavearr commented Oct 14, 2026

Copy link
Copy Markdown

I take back what I asked about the kintex7 order.

prjxray has utils/sort_db.py to put the db files in a canonical order. On 75825f2 it leaves the DSP masks of artix7, spartan7 and zynq7 unchanged, because they already use its order. It changes the kintex7 ones, which mergedb last wrote in plain string order. Your two kintex7 mask files are exactly what sort_db.py writes. So the +340/+448 is the tool's own order, the one the other families already have.

I also ran the fuzzer from prjxray#31 (cbe9bed6) with pushdb into a copy of 75825f2, N=8, on one part of each family: xc7a100t, xc7s25, xc7z020 and xc7k70t. The segbits it writes are the rows of this PR, row for row. On artix7, spartan7 and zynq7, pushdb followed by sort_db.py gives your segbits files byte for byte. pushdb on its own writes the kintex7 masks in string order, and sort_db.py turns them into your order. pushdb also adds mask bits in the routing frames of the tile (+720 and +603 on artix7) and rewrites the origin of the rows it solved again. The nextpnr chipdb of the 11 dies we ship is byte-identical with and without this PR, so I would keep the PR as it is: the two rows and the two mask bits per side.

checkdb fails the same six jobs as master (kintex7 x5 and virtex7), with the same assertion in each, so it is unrelated.

@cavearr
cavearr merged commit 4052440 into openXC7:master Oct 14, 2026
17 of 23 checks passed
@cavearr

cavearr commented Oct 14, 2026

Copy link
Copy Markdown

Merged, thank you. nextpnr#95 is the last one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants