From fc87eae92b38788bccf3c127f7ab241210bb3d45 Mon Sep 17 00:00:00 2001 From: Dmitrii Vasilev Date: Mon, 10 Aug 2026 23:27:17 +0700 Subject: [PATCH 1/2] regression: guard the two IOB divergences from Vivado, and add reject.txt Adds iob-lvcmos33-drive-slew, guarding openXC7/nextpnr-xilinx#120: an LVCMOS33 output at the default drive was getting the I12_I8 bit pattern where Vivado emits I12_I16, and every input-only pad was getting output SLEW.SLOW bits. This is the first case here whose criterion is not "the flow used to fall over". Both bugs produced a valid bitstream that merely programmed the pad differently, so the expected values are transcribed from the four Vivado-built bitstreams prjxray-db already ships in artix7/harness/, not from our own output. Their design.dcp files carry Vivado's top_late.xdc, which constrains nothing but PACKAGE_PIN and IOSTANDARD LVCMOS33, so the comparison is like-for-like. It also needs a check the runner did not have. A fix that makes us stop emitting a wrong bit is invisible to expect.txt, because the absence of a bit is not a string you can grep for. So cases may now carry a reject.txt, same format, and fail if any pattern matches. This case uses both: expect.txt for the drive pattern that should now appear on the output pad, reject.txt for the wrong pattern and for the slew bits that should no longer appear on the input pad. Checked against unpatched nextpnr-xilinx, where it fails with both divergences reproduced, so it is a real guard rather than a tautology. Suite on 0.9.2 + the #120 patch, chipdb xc7a200tfbg484-2: clock-srcc-bufg ok bram-sdp-unused-port ok bufg-fabric-driven ok config-primitive-startupe2 ok iddr-four-iff-flops ok iob-lvcmos33-drive-slew ok --- regression/README.md | 47 +++++++++++++++++++ regression/iob-lvcmos33-drive-slew/expect.txt | 2 + regression/iob-lvcmos33-drive-slew/reject.txt | 2 + regression/iob-lvcmos33-drive-slew/top.v | 21 +++++++++ regression/iob-lvcmos33-drive-slew/top.xdc | 7 +++ regression/run.sh | 14 +++++- 6 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 regression/iob-lvcmos33-drive-slew/expect.txt create mode 100644 regression/iob-lvcmos33-drive-slew/reject.txt create mode 100644 regression/iob-lvcmos33-drive-slew/top.v create mode 100644 regression/iob-lvcmos33-drive-slew/top.xdc diff --git a/regression/README.md b/regression/README.md index 4a38ae1..65f34e7 100644 --- a/regression/README.md +++ b/regression/README.md @@ -14,6 +14,53 @@ One directory per fixed bug. Each design **failed before its patch** and builds | `bufg-fabric-driven` | #111 | the placer aborted instead of pre-placing a BUFG driven from the fabric, so any design that divides or gates a clock in logic and re-buffers it failed to place | | `config-primitive-startupe2` | #113 | the single-site configuration primitives had no pre-placement, so instantiating `STARTUPE2` failed to place | | `iddr-four-iff-flops` | #115 | only Q1/Q2 of the four-flop IFF were initialised; on silicon the outputs then read Q1=0, Q2=1 despite both being programmed INIT=0 | +| `iob-lvcmos33-drive-slew` | [nextpnr-xilinx#120](https://github.com/openXC7/nextpnr-xilinx/pull/120) | an LVCMOS33 output at the default drive got the `I12_I8` bit pattern where Vivado emits `I12_I16`, and every input-only pad got output `SLEW.SLOW` bits | + +## `reject.txt`: patterns that must *not* appear + +`expect.txt` cannot guard a fix that makes us **stop** emitting a wrong bit — the bit's +absence is not a string you can grep for. Those cases carry a `reject.txt` instead, with +the same one-regex-per-line format; the case fails if any of them matches. + +`iob-lvcmos33-drive-slew` uses both: `expect.txt` for the drive pattern that should now +appear on the output pad, `reject.txt` for the wrong drive pattern and for the slew bits +that should no longer appear on the input pad. + +## Where the `iob-lvcmos33-drive-slew` expectations come from + +This is the first case whose criterion is not "the flow used to fall over". Both bugs +produced a perfectly valid bitstream that merely programmed the pad differently from +Vivado, so the expected values are transcribed from **vendor output**, not from ours. + +`prjxray-db` ships four Vivado-built bitstreams together with their design checkpoints: + +``` +artix7/harness/arty-a7/{swbut,uart,pmod}/design.bit +artix7/harness/basys3/swbut/design.bit +``` + +`prjxray/utils/bit2fasm.py` turns them back into FASM, and each `.dcp` contains Vivado's +own `top_late.xdc`, which constrains nothing but `PACKAGE_PIN` and `IOSTANDARD LVCMOS33` +— so every pad sits at Vivado's defaults and the comparison is like-for-like. Across all +four, 35 output pads: + +* the drive pattern is `LVCMOS33_LVTTL.DRIVE.I12_I16` on every one, and `I12_I8` does not + occur once — note that `prjxray-db` names *both* patterns as covering drive 12, which + cannot both be right; the vendor bitstreams are what break the tie; +* `SLEW.SLOW` appears exactly as many times as there are output pads, and never on an + input pad. + +Two classes of difference are artefacts of the comparison and must be filtered out before +reading anything into a FASM-to-disassembled-FASM diff: + +* features whose segbits are **all negated** (`IN_TERM.NONE`, `SLEW.FAST`, + `IDELMUXE3.P1`, `ISERDES.MODE.MASTER`, …) set no bits at all, so emitting them or not + cannot change the bitstream; +* `always` pseudo-pips from `ppips_*.db` have no bits either, so `bit2fasm` can never + recover them from a bitstream even though nextpnr emits them. + +In the arty-a7/swbut comparison those two classes accounted for 84 and 42 lines +respectively; filtering them left exactly the two differences above. ## Running diff --git a/regression/iob-lvcmos33-drive-slew/expect.txt b/regression/iob-lvcmos33-drive-slew/expect.txt new file mode 100644 index 0000000..4f8d306 --- /dev/null +++ b/regression/iob-lvcmos33-drive-slew/expect.txt @@ -0,0 +1,2 @@ +LIOB33_X0Y233\.IOB_Y1\.LVCMOS33_LVTTL\.DRIVE\.I12_I16 +LIOB33_X0Y233\.IOB_Y1\..*SLEW\.SLOW diff --git a/regression/iob-lvcmos33-drive-slew/reject.txt b/regression/iob-lvcmos33-drive-slew/reject.txt new file mode 100644 index 0000000..edf7d65 --- /dev/null +++ b/regression/iob-lvcmos33-drive-slew/reject.txt @@ -0,0 +1,2 @@ +DRIVE\.I12_I8 +RIOB33_X105Y115\..*SLEW\.SLOW diff --git a/regression/iob-lvcmos33-drive-slew/top.v b/regression/iob-lvcmos33-drive-slew/top.v new file mode 100644 index 0000000..3d1f9bd --- /dev/null +++ b/regression/iob-lvcmos33-drive-slew/top.v @@ -0,0 +1,21 @@ +// Regression for the two IOB divergences found by diffing against the +// Vivado-built references committed in prjxray-db/artix7/harness/. +// +// 1. an LVCMOS33 output at the default drive must use the I12_I16 bit +// pattern (Vivado's), not I12_I8 +// 2. SLEW is an output-driver property: an input-only pad must not get +// SLEW.SLOW bits +// +// clk is an input-only pad and led an output, so one design exercises both +// rules. The expectations are transcribed from a vendor bitstream (see the +// README), not from our own output. +module top ( + input wire clk, + output reg led +); + reg [23:0] ctr = 0; + always @(posedge clk) begin + ctr <= ctr + 1; + led <= ctr[23]; + end +endmodule diff --git a/regression/iob-lvcmos33-drive-slew/top.xdc b/regression/iob-lvcmos33-drive-slew/top.xdc new file mode 100644 index 0000000..811826d --- /dev/null +++ b/regression/iob-lvcmos33-drive-slew/top.xdc @@ -0,0 +1,7 @@ +# AX7203 (xc7a200tfbg484-2) pins, same set the other cases in this directory use. +# T6 is an input-only pad here, B13 an output -- the two roles the case checks. +create_clock -period 10.000 -name clk [get_ports clk] +set_property PACKAGE_PIN T6 [get_ports clk] +set_property IOSTANDARD LVCMOS33 [get_ports clk] +set_property PACKAGE_PIN B13 [get_ports led] +set_property IOSTANDARD LVCMOS33 [get_ports led] diff --git a/regression/run.sh b/regression/run.sh index 5a25de5..c6ad546 100755 --- a/regression/run.sh +++ b/regression/run.sh @@ -15,7 +15,7 @@ HERE="$(cd "$(dirname "$0")" && pwd)" cases=("$@"); [ ${#cases[@]} -eq 0 ] && cases=(clock-srcc-bufg bram-sdp-unused-port \ bufg-fabric-driven config-primitive-startupe2 \ - iddr-four-iff-flops) + iddr-four-iff-flops iob-lvcmos33-drive-slew) fail=0 for c in "${cases[@]}"; do d="$HERE/$c" @@ -40,6 +40,18 @@ for c in "${cases[@]}"; do done < "$d/expect.txt" [ "$miss" -eq 0 ] || { fail=1; continue; } fi + # The mirror of expect.txt: patterns that must NOT appear. A fix that stops + # emitting a wrong bit is invisible to a "must contain" check, so those cases + # carry a reject.txt instead. + if [ -f "$d/reject.txt" ]; then + hit=0 + while read -r pat; do + [ -z "$pat" ] && continue + if grep -qE -- "$pat" "$d/top.fasm"; then + printf ' %-26s FAIL (fasm contains: %s)\n' "$c" "$pat"; hit=1; fi + done < "$d/reject.txt" + [ "$hit" -eq 0 ] || { fail=1; continue; } + fi printf ' %-26s ok (%s)\n' "$c" "$(du -h "$d/top.fasm" | cut -f1)" done exit $fail From 848008e9a5461e8333893f2917c1967a5b85c52a Mon Sep 17 00:00:00 2001 From: Dmitrii Vasilev Date: Tue, 11 Aug 2026 00:49:43 +0700 Subject: [PATCH 2/2] ci: actually run the regression suite The suite was moved here from nextpnr-xilinx in openXC7/nextpnr-xilinx#118 because this repo has the chipdb and working CI -- but nothing was ever wired up to invoke it. `grep -rn "run.sh" .github/workflows/` returns nothing, so six cases guarding six fixed bugs have been running nowhere, and the move did not achieve what it was for. Adds a `regression` job that reuses the chipdb-artix7 artifact stage 1 already produces. The cases need only yosys, nextpnr-xilinx and one chipdb, and they stop at the FASM, so this costs minutes rather than the hours the demo matrix takes. The job asserts the chipdb is present before running. Without that check an absent artifact would fail every case for the wrong reason, which on the job list looks exactly like a real regression -- the same indistinguishable-failure problem .DELETE_ON_ERROR was added for in #13. Note this runs the pinned toolchain's nextpnr-xilinx, which is correct here: this repo tests the released flow. The equivalent gate on the nextpnr-xilinx side has the opposite requirement and is currently not meeting it -- see openXC7/nextpnr-xilinx#125. --- .github/workflows/smoke.yml | 40 +++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index 3006988..160cbed 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -95,6 +95,46 @@ jobs: path: ${{ steps.build.outputs.stage }} retention-days: 3 + # --------------------------------------------------------------------- + # Stage 1b: the nextpnr-xilinx regression suite. + # + # These cases were moved here from nextpnr-xilinx (that repo has no + # chipdb and no working CI) in openXC7/nextpnr-xilinx#118 -- but nothing + # was ever wired up to run them, so six cases guarding six fixed bugs + # have been executing nowhere. This job runs them. + # + # They need only yosys + nextpnr-xilinx + one artix7 chipdb, and stop at + # the FASM, so this is minutes rather than the hours the demo matrix takes. + # --------------------------------------------------------------------- + regression: + name: regression-suite + needs: chipdb + runs-on: ubuntu-latest + timeout-minutes: 60 + steps: + - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + repository: ${{ env.TOOLCHAIN_NIX_REPO }} + path: toolchain-nix + - uses: actions/download-artifact@v4 + with: + name: chipdb-artix7 + path: chipdb + - uses: DeterminateSystems/nix-installer-action@v14 + - uses: DeterminateSystems/magic-nix-cache-action@v6 + + - name: Run the regression suite + run: | + ls -la chipdb + nix develop ./toolchain-nix --command bash -euxo pipefail <<'EOF' + CHIPDB="$GITHUB_WORKSPACE/chipdb/xc7a200tfbg484.bin" + # An absent chipdb would make every case fail for the wrong + # reason, which reads the same as a real regression. Say so. + test -s "$CHIPDB" || { echo "::error::no artix7 chipdb at $CHIPDB"; exit 1; } + CHIPDB="$CHIPDB" regression/run.sh + EOF + # --------------------------------------------------------------------- # Stage 2a: one job per demo project. The GitHub job list IS the # overview: green = the project builds to .bit, red = it does not.