Expression is ambiguous in quartus (integer/universal integer). - #1
Open
nwrkbiz wants to merge 1 commit into
Open
Expression is ambiguous in quartus (integer/universal integer).#1nwrkbiz wants to merge 1 commit into
nwrkbiz wants to merge 1 commit into
Conversation
Bluebugs
referenced
this pull request
in Bluebugs/jcore-cpu
May 27, 2026
First independent review pass after Phase 4 cutover (49aaf11) caught nine issues — four correctness, two build, three documentation. All addressed here; final regression passes end-to-end on both decoders. Correctness fixes: Review #1 — direct decoder: NOT_BUSY default for SigMacBusy. SignalDefault(SigMacBusy) returned ("", false), so the direct decoder's QMC grouping never saw a NOT_BUSY arm for the ~140 non-MAC slots. Generated VHDL emitted with cond... select mac_busy <= EX_NOT_STALL when others; instead of ... <= NOT_BUSY when others; Hidden from the 20-LED regression because the testrom doesn't exercise the MAC stall path. Confirmed against Clojure: NOT_BUSY is the first literal of mac_busy_t in interface.clj line 189. Fix: add `case SigMacBusy: return "NOT_BUSY", true`. Also adds TestSignalDefaultCoversAllSignals — explicit nillable + default coverage of every entry in microcode.AllSignals, with the set transcribed from Clojure's :nillable-outputs. This is the test that would have caught the bug originally. Review j-core#3 — BuildDirect determinism. The grouped[(sig,val)] → []LogicMap map was populated by iterating per-slot effectiveAM AssignMaps; Go map iteration order is randomized, so different runs could append the same LogicMaps in different orders, changing QMC tie-breaking and shifting imp_bit numbering — even though the generated VHDL stayed semantically equivalent. Fix sorts the keys before iterating. Verified by running cpugen twice into separate temp dirs; post-fix output is byte-identical across runs. Review #8 — TestBuildDirectSemanticSpotChecks regression guards. Asserts mac_busy is a 3-arm mux with NOT_BUSY default, and that at least one direct-decoder output references t_bcc — guarding the classes of bugs fixed in 44b1481 (T/NT routing), 60e2a53 (per-signal defaults), and review #1 above. Build fix: Review j-core#2 — decode/Makefile: drop decode_core.vhd from GENERATED. decode_core.vhd is the C-preprocessor output of decode_core.vhm, produced by sim/Makefile — NOT by cpugen. Including it in GENERATED meant "make -C decode clean" deleted it but "make -C decode generate" never restored it, leaving the simulator unable to compile. Verified: only the .vhm is tracked in git; internal/emit/emit.go does not produce a decode_core.vhd. Documentation: Review j-core#4 — drop stale m5-verification.md from docs/. The file described seven generator bugs as "open" while the same bugs had been fixed in commits 414e16f and the M5 squash (now collapsed into one M5 verification pass). Folded into the SigMacBusy commit due to staging order; no separate action remained. Review j-core#5 — README worked example: register the new instruction in csvInstrOrder. A new TOML instruction not added to csvInstrOrder triggers a Build error ("missing from csvInstrOrder"). This was missing from the worked example, so users hit a confusing error immediately after step 2. New step 3 explains the slice, where to insert (append, to preserve existing addresses), and quotes the exact error message. Review #6 — README worked example uses real TOML. Previous block used xbus="0" and ybus="Rm" with format="mn", which doesn't match any real instruction and made the register routing rules unclear. Rewritten to mirror the actual "SUB Rm, Rn" entry from spec/arithmetic.toml byte-for-byte; only name, opcode, and operation differ. Verified end-to-end by temporarily appending the example, regenerating, and removing it. Also normalizes the instruction name spelling to NEGX (no dot) consistently in TOML, prose, and the example block. Review #7 — TestSignalDefaultCoversAllSignals (included with the SigMacBusy fix above; the test that would have caught review #1). Review #9 — document the LED-write timing divergence. New section in decode/gen-go/README.md noting that the Go direct decoder produces LED writes 90 ns – 3.4 µs later than the Clojure baseline at the same opcode, due to semantically-equivalent QMC tie-breaking. Not a regression; clearly distinguished from the LED sequence (which IS regression-checked). (Squashed from 4 commits: 6efae2d, daea466, 70ab6ca, 2022319.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bluebugs
referenced
this pull request
in Bluebugs/jcore-cpu
May 27, 2026
…CIFICATION.md refs DEVELOPERS.md now documents the Go 1.26+ decoder generator workflow (replaces dead Clojure/Leiningen narrative). Install blocks updated for Debian/Arch/macOS; new optional-tools section with yosys+ghdl-yosys-plugin build recipe, Nangate45 Liberty curl recipe, /tmp non-persistence warning, and NANGATE_LIB override note. "Modifying the Instruction Set" workflow swapped from 'lein run' to 'make -C decode generate'. "Regenerating the Instruction Decoder" section fully rewritten. CLAUDE.md Go version bumped to 1.26+, SPECIFICATION.md reference removed from repo tree (file was archived), .ods description corrected to "legacy; archived". decode/gen-go/README.md removes dead SPECIFICATION.md reference; "Known gaps" section rewritten to clarify Steps 7/8 (synth/STA) are operational and defer only FPGA hardware boards, vendor tools, and silicon. differential_test.go adds coverage-gap documentation for nillable signals. Fixes #1, j-core#5, #7, #8, #14, #15 from full-branch review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bluebugs
referenced
this pull request
in Bluebugs/jcore-cpu
May 28, 2026
First independent review pass after Phase 4 cutover (49aaf11) caught nine issues — four correctness, two build, three documentation. All addressed here; final regression passes end-to-end on both decoders. Correctness fixes: Review #1 — direct decoder: NOT_BUSY default for SigMacBusy. SignalDefault(SigMacBusy) returned ("", false), so the direct decoder's QMC grouping never saw a NOT_BUSY arm for the ~140 non-MAC slots. Generated VHDL emitted with cond... select mac_busy <= EX_NOT_STALL when others; instead of ... <= NOT_BUSY when others; Hidden from the 20-LED regression because the testrom doesn't exercise the MAC stall path. Confirmed against Clojure: NOT_BUSY is the first literal of mac_busy_t in interface.clj line 189. Fix: add `case SigMacBusy: return "NOT_BUSY", true`. Also adds TestSignalDefaultCoversAllSignals — explicit nillable + default coverage of every entry in microcode.AllSignals, with the set transcribed from Clojure's :nillable-outputs. This is the test that would have caught the bug originally. Review j-core#3 — BuildDirect determinism. The grouped[(sig,val)] → []LogicMap map was populated by iterating per-slot effectiveAM AssignMaps; Go map iteration order is randomized, so different runs could append the same LogicMaps in different orders, changing QMC tie-breaking and shifting imp_bit numbering — even though the generated VHDL stayed semantically equivalent. Fix sorts the keys before iterating. Verified by running cpugen twice into separate temp dirs; post-fix output is byte-identical across runs. Review #8 — TestBuildDirectSemanticSpotChecks regression guards. Asserts mac_busy is a 3-arm mux with NOT_BUSY default, and that at least one direct-decoder output references t_bcc — guarding the classes of bugs fixed in 44b1481 (T/NT routing), 60e2a53 (per-signal defaults), and review #1 above. Build fix: Review j-core#2 — decode/Makefile: drop decode_core.vhd from GENERATED. decode_core.vhd is the C-preprocessor output of decode_core.vhm, produced by sim/Makefile — NOT by cpugen. Including it in GENERATED meant "make -C decode clean" deleted it but "make -C decode generate" never restored it, leaving the simulator unable to compile. Verified: only the .vhm is tracked in git; internal/emit/emit.go does not produce a decode_core.vhd. Documentation: Review j-core#4 — drop stale m5-verification.md from docs/. The file described seven generator bugs as "open" while the same bugs had been fixed in commits 414e16f and the M5 squash (now collapsed into one M5 verification pass). Folded into the SigMacBusy commit due to staging order; no separate action remained. Review j-core#5 — README worked example: register the new instruction in csvInstrOrder. A new TOML instruction not added to csvInstrOrder triggers a Build error ("missing from csvInstrOrder"). This was missing from the worked example, so users hit a confusing error immediately after step 2. New step 3 explains the slice, where to insert (append, to preserve existing addresses), and quotes the exact error message. Review #6 — README worked example uses real TOML. Previous block used xbus="0" and ybus="Rm" with format="mn", which doesn't match any real instruction and made the register routing rules unclear. Rewritten to mirror the actual "SUB Rm, Rn" entry from spec/arithmetic.toml byte-for-byte; only name, opcode, and operation differ. Verified end-to-end by temporarily appending the example, regenerating, and removing it. Also normalizes the instruction name spelling to NEGX (no dot) consistently in TOML, prose, and the example block. Review #7 — TestSignalDefaultCoversAllSignals (included with the SigMacBusy fix above; the test that would have caught review #1). Review #9 — document the LED-write timing divergence. New section in decode/gen-go/README.md noting that the Go direct decoder produces LED writes 90 ns – 3.4 µs later than the Clojure baseline at the same opcode, due to semantically-equivalent QMC tie-breaking. Not a regression; clearly distinguished from the LED sequence (which IS regression-checked). (Squashed from 4 commits: 6efae2d, daea466, 70ab6ca, 2022319.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bluebugs
referenced
this pull request
in Bluebugs/jcore-cpu
May 28, 2026
…CIFICATION.md refs DEVELOPERS.md now documents the Go 1.26+ decoder generator workflow (replaces dead Clojure/Leiningen narrative). Install blocks updated for Debian/Arch/macOS; new optional-tools section with yosys+ghdl-yosys-plugin build recipe, Nangate45 Liberty curl recipe, /tmp non-persistence warning, and NANGATE_LIB override note. "Modifying the Instruction Set" workflow swapped from 'lein run' to 'make -C decode generate'. "Regenerating the Instruction Decoder" section fully rewritten. CLAUDE.md Go version bumped to 1.26+, SPECIFICATION.md reference removed from repo tree (file was archived), .ods description corrected to "legacy; archived". decode/gen-go/README.md removes dead SPECIFICATION.md reference; "Known gaps" section rewritten to clarify Steps 7/8 (synth/STA) are operational and defer only FPGA hardware boards, vendor tools, and silicon. differential_test.go adds coverage-gap documentation for nillable signals. Fixes #1, j-core#5, #7, #8, #14, #15 from full-branch review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bluebugs
referenced
this pull request
in Bluebugs/jcore-cpu
May 28, 2026
First independent review pass after Phase 4 cutover (49aaf11) caught nine issues — four correctness, two build, three documentation. All addressed here; final regression passes end-to-end on both decoders. Correctness fixes: Review #1 — direct decoder: NOT_BUSY default for SigMacBusy. SignalDefault(SigMacBusy) returned ("", false), so the direct decoder's QMC grouping never saw a NOT_BUSY arm for the ~140 non-MAC slots. Generated VHDL emitted with cond... select mac_busy <= EX_NOT_STALL when others; instead of ... <= NOT_BUSY when others; Hidden from the 20-LED regression because the testrom doesn't exercise the MAC stall path. Confirmed against Clojure: NOT_BUSY is the first literal of mac_busy_t in interface.clj line 189. Fix: add `case SigMacBusy: return "NOT_BUSY", true`. Also adds TestSignalDefaultCoversAllSignals — explicit nillable + default coverage of every entry in microcode.AllSignals, with the set transcribed from Clojure's :nillable-outputs. This is the test that would have caught the bug originally. Review j-core#3 — BuildDirect determinism. The grouped[(sig,val)] → []LogicMap map was populated by iterating per-slot effectiveAM AssignMaps; Go map iteration order is randomized, so different runs could append the same LogicMaps in different orders, changing QMC tie-breaking and shifting imp_bit numbering — even though the generated VHDL stayed semantically equivalent. Fix sorts the keys before iterating. Verified by running cpugen twice into separate temp dirs; post-fix output is byte-identical across runs. Review #8 — TestBuildDirectSemanticSpotChecks regression guards. Asserts mac_busy is a 3-arm mux with NOT_BUSY default, and that at least one direct-decoder output references t_bcc — guarding the classes of bugs fixed in 44b1481 (T/NT routing), 60e2a53 (per-signal defaults), and review #1 above. Build fix: Review j-core#2 — decode/Makefile: drop decode_core.vhd from GENERATED. decode_core.vhd is the C-preprocessor output of decode_core.vhm, produced by sim/Makefile — NOT by cpugen. Including it in GENERATED meant "make -C decode clean" deleted it but "make -C decode generate" never restored it, leaving the simulator unable to compile. Verified: only the .vhm is tracked in git; internal/emit/emit.go does not produce a decode_core.vhd. Documentation: Review j-core#4 — drop stale m5-verification.md from docs/. The file described seven generator bugs as "open" while the same bugs had been fixed in commits 414e16f and the M5 squash (now collapsed into one M5 verification pass). Folded into the SigMacBusy commit due to staging order; no separate action remained. Review j-core#5 — README worked example: register the new instruction in csvInstrOrder. A new TOML instruction not added to csvInstrOrder triggers a Build error ("missing from csvInstrOrder"). This was missing from the worked example, so users hit a confusing error immediately after step 2. New step 3 explains the slice, where to insert (append, to preserve existing addresses), and quotes the exact error message. Review #6 — README worked example uses real TOML. Previous block used xbus="0" and ybus="Rm" with format="mn", which doesn't match any real instruction and made the register routing rules unclear. Rewritten to mirror the actual "SUB Rm, Rn" entry from spec/arithmetic.toml byte-for-byte; only name, opcode, and operation differ. Verified end-to-end by temporarily appending the example, regenerating, and removing it. Also normalizes the instruction name spelling to NEGX (no dot) consistently in TOML, prose, and the example block. Review #7 — TestSignalDefaultCoversAllSignals (included with the SigMacBusy fix above; the test that would have caught review #1). Review #9 — document the LED-write timing divergence. New section in decode/gen-go/README.md noting that the Go direct decoder produces LED writes 90 ns – 3.4 µs later than the Clojure baseline at the same opcode, due to semantically-equivalent QMC tie-breaking. Not a regression; clearly distinguished from the LED sequence (which IS regression-checked). (Squashed from 4 commits: 6efae2d, daea466, 70ab6ca, 2022319.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bluebugs
referenced
this pull request
in Bluebugs/jcore-cpu
May 28, 2026
…CIFICATION.md refs DEVELOPERS.md now documents the Go 1.26+ decoder generator workflow (replaces dead Clojure/Leiningen narrative). Install blocks updated for Debian/Arch/macOS; new optional-tools section with yosys+ghdl-yosys-plugin build recipe, Nangate45 Liberty curl recipe, /tmp non-persistence warning, and NANGATE_LIB override note. "Modifying the Instruction Set" workflow swapped from 'lein run' to 'make -C decode generate'. "Regenerating the Instruction Decoder" section fully rewritten. CLAUDE.md Go version bumped to 1.26+, SPECIFICATION.md reference removed from repo tree (file was archived), .ods description corrected to "legacy; archived". decode/gen-go/README.md removes dead SPECIFICATION.md reference; "Known gaps" section rewritten to clarify Steps 7/8 (synth/STA) are operational and defer only FPGA hardware boards, vendor tools, and silicon. differential_test.go adds coverage-gap documentation for nillable signals. Fixes #1, j-core#5, #7, #8, #14, #15 from full-branch review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Problem arises when using Intel/Altera quartus prime for synthesis.