Repository navigation
Conversation
…module Register allocator: - release a block's registers after its last data consumer, including values produced outside a loop and read inside it - keep pre-initialized loop values in their own register and support explicit loop accumulator bindings and exported loop results - route each consumer input from the output port it is wired to and refuse connections to ports that own no register - record register-pool exhaustion per block so validation can name it - reserve user-typed registers that blocks emit verbatim and report overlapping ones - give multi-result macros one destination register per output - keep 64-bit (two register) sources and outputs Static module and templates: - report allocation failures, pinned-register conflicts, unconnected required inputs, invalid port connections, loop/container boundary violations and macro arity mismatches at validation time - program CTPPR0 so c28 points at shared RAM when a block uses c28, and drive SPI controller CS pins to their idle level, at every entry into generated code - warn in the GUI when a GPO/GPI pad is shared or a GPO has two drivers - gate blocks that need a hardware widget on the device capability table - define TEMP_REG3/TEMP_REG4 (R26/R27) for macros - expand multi-line instructions in the simulator path New shared scripts: pru_reserved_names.js (clpru reserved identifiers), pru_spi_timing.js (SPI delay limits and IRAM budget), pru_device_caps.js (per-SoC hardware widgets), block_control_flow_data.js.
- CRC16/CRC32: scale the table index to the entry size before the load (the byte offset does not fit the LBBO offset operand); 13 cycles - Access Lookup Table: scale the index for ushort/uint tables (7 cycles) - PRU GPI: build the bit mask in the result register so pins above 7 work; 3 cycles, 4-byte output - PRU SPI Read/Transfer: clear the result register before sampling so short packets are zero-extended; reserve the whole register for 17-24 bit packets; one extra overhead cycle - PRU SPI Read/Write/Transfer: refuse CS/data delays above 256 cycles, warn when the SPI blocks exceed the PRU IRAM budget - If/Else: an unconnected true/false branch is an error (the generated code would reference an undefined label) - Load Constant: optional explicit output width - Loop: reject a hardware LOOP nested in a finite loop body; add exported result blocks and accumulator bindings - Delay: document the exact cycle count and the nesting restriction - Memory Access: warn when two blocks share one auto-created Memory Variable - CRC, Lookup Table, Memory Variable, Group, If/Else: reject symbols that clash with clpru reserved identifiers - PRU GPI/GPO and SPI blocks: declare the pins they use (getPinUsage)
- tests/test_syscfg_add_instance.py: every block module adds an instance through the SysConfig CLI (skipped when SysConfig is not installed) - tests/test_fail_closed_gating.py: the device-capability gate fails closed on every degraded path - tests/test_shared_memvar_rename.py, tests/test_conditional_unconnected_branch.py - scripts/static_module_validate_runner.js, scripts/block_validate_runner.js, scripts/shared_memvar_runner.js: run block validators under node
… test The register allocator now releases a register once its last reader has run, so main.asm can no longer rely on a released register still holding its value. Name the registers once in main.asm, and keep the received frame live on the receiving core by reading it into an otherwise unused block output. Add a firmware test that runs the example on the simulator for both boards and both cores, checking the stored data byte and the CRC match/mismatch result.
…es not exist; neutral test wording
The MODE1 LSB loop advanced its bit counter in the low half, where the delay formulas assume none, so SCLK ran one cycle short high and one cycle long low. The MODE3 LSB loop sampled a 1 bit one instruction slower than a 0 bit, so its high half grew by a cycle on every 1. Both variants now count in the half the MSB variant counts in, and both sample paths take two instructions.
The Memory Variable block reserves buffers in .udmem (data RAM) and .usmem (shared RAM). With no rule for either section clpru placed them in instruction RAM after .text. Each example linker file now places .udmem in the data RAM its .bss uses and .usmem in PRU_SHAREDMEM, and a test checks every tracked linker file.
|
/agentic_describe |
|
/agentic_review |
PR Summary by QodoAdd shared PRU block infrastructure and fix generated firmware
AI Description
Diagram
High-Level Assessment
Files changed (78)
|
Code Review by QodoView medium (2)
1. Missing pad data suppresses sharing warnings
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
|
||
| // 2. Raw JSON device data structure (ICSSM devices, stubs and test harnesses). ICSSG pads carry a | ||
| // PRGx_PRUy_GPOn + PRGx_PRUy_GPIn pair; ICSSM pads carry one two-way PRx_PRUy_GPIOn signal. | ||
| const infos = deviceData.pinCommonInfos || {}; |
There was a problem hiding this comment.
1. Missing pad data suppresses sharing warnings 🐞 Bug ◔ Observability
getDevicePadSharing returns an empty Set when device data has neither a usable runtime pin interface nor pinCommonInfos, treating unavailable metadata as evidence that no pads are shared. When a design has GPO and GPI claims under that condition, validation emits neither a collision warning nor the warning that pad sharing could not be checked.
Agent Prompt
## Issue description
Unavailable device pin metadata is treated as a valid empty sharing map, so designs with GPO and GPI claims receive no warning that sharing could not be checked.
## Fix Focus Areas
- .metadata/sysconfig/.meta/pru_blocks/common/pru_blocks_static_module.syscfg.js[1225-1279]
- .metadata/sysconfig/.meta/pru_blocks/common/pru_blocks_static_module.syscfg.js[1373-1385]
## Recommended Fix
Return `null` when none of the supported pin-data sources is available, while retaining an empty set when available data genuinely contains no shared pads. Add a validation case with GPO and GPI claims and missing pin metadata.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Fixed in 84aaee1: getDevicePadSharing now returns null when the device data has no usable pin source, so the "pad sharing could not be checked" warning is emitted; an empty set is kept only when available data has no shared pad. Added tests for missing, empty, shared-pad and no-shared-pad data.
| const width = parseInt(instance["output" + output + "Size"] || maxBytesUsed); | ||
| const extraOffset = getFreeByteOffset(width); | ||
| if (extraOffset === -1) { | ||
| recordAllocationFailure(instance, width, "result output" + output); | ||
| return -1; | ||
| } | ||
| moduleInstanceRegisterMap[instanceName]["byteOffsetOfOutput" + output] = extraOffset; | ||
| moduleInstanceRegisterMap[instanceName]["numOfBytesReqByOutput" + output] = width; | ||
| macroOutputRegisters.push(getPruRegister(extraOffset, width)); |
There was a problem hiding this comment.
3. Wide secondary outputs lose their upper half 🐞 Bug ≡ Correctness
For multi-output macros, the allocator reserves width bytes for each outputN with N ≥ 2, but it emits only getPruRegister(extraOffset, width). For widths above 4 bytes that returns just the starting register. Consumers wired to those ports go through sourceRegisterToken(), which also returns a single token, whereas output1 gets explicit lo/hi register pairs, so any 64-bit second output passes only its low 32 bits to the macro and to downstream blocks.
Agent Prompt
## Issue description
Extra outputs (output2 and up) wider than 4 bytes are allocated 8 bytes but passed as one register, both as macro destinations and to their consumers.
## Fix Focus Areas
- .metadata/sysconfig/.meta/pru_blocks/common/register_allocation/pru_register_allocator.js[1396-1406]
- .metadata/sysconfig/.meta/pru_blocks/common/register_allocation/pru_register_allocator.js[1193-1196]
## Recommended Fix
When `width > 4`, push `getPruRegister(extraOffset,4) + ", " + getPruRegister(extraOffset+4,4)`. In `sourceRegisterToken` (or at the call site), emit lo and hi tokens when `alloc.numBytes > 4`, the same way the output1 path does. Alternatively, reject widths above 4 for non-output1 ports with a validation error.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Fixed in bb61cac: for output2 and up wider than 4 bytes the macro destination and the consumer's source now carry both registers (lo, hi), as output1 already does. Added an allocator test.
| } else if (args.includes('--inst-file')) { | ||
| const idx = args.indexOf('--inst-file'); | ||
| const fp = args[idx + 1]; | ||
| inst = JSON.parse(fs.readFileSync(fp, 'utf8')); |
There was a problem hiding this comment.
2. Validator script ignores the instance-file flag 🐞 Bug ≡ Correctness
main() in scripts/block_validate_runner.js recognizes only --block and --inst, so its positional fallback assigns --inst-file to blockPath or instJson before the file-reading branch can run. With --inst-file f.json first, it looks for a block named --inst-file and exits with “Block not found”; after a block path, it tries to parse --inst-file as JSON instead of validating the instance in the file.
Agent Prompt
## Issue description
`--inst-file <path>` is consumed as a positional argument, preventing the runner from reading and validating the instance file.
## Fix Focus Areas
- scripts/block_validate_runner.js[277-301]
## Recommended Fix
Handle `--inst-file` and its following path in the argument loop before the positional fallbacks. After the loop, read and parse the specified file into `inst` with try/catch rather than parsing the option name as JSON; add the flag to the usage text and a CLI test for a file-based invocation.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Fixed in 498fcb0: --inst-file is parsed in the argument loop, the file is read and parsed (with an error and exit code 2 if it cannot be), and it is listed in the usage text. Added CLI tests for both argument orders.
The option was not recognised in the argument loop, so its name was taken as the block path or the instance JSON and the file was never read. It is now parsed with its path, read and parsed with a clear error on failure, and listed in the usage text.
getDevicePadSharing returned an empty set when the device data had neither a usable runtime pin interface nor pin common info, which the validator read as "no pad is shared" and so emitted neither a collision warning nor the "could not be checked" warning. It now returns null in that case, keeping an empty set only when available data holds no shared pad.
…ry ports A multi-output macro reserved the full width for output2 and up but emitted only the first register of the pair, and consumers wired to such a port read only that register, so a 64-bit second result lost its upper half. The macro destination and the consumer's source token now carry both registers, as output1 already did.
TEMP_REG3 and TEMP_REG4 alias R26 and R27, which were still in the allocatable pool, so a macro writing them could overwrite a live value or its own result. Before allocation the pool now drops R26 and/or R27 when any block's macro text names TEMP_REG3 and/or TEMP_REG4; designs that use neither keep the full 28-register pool. The summary comments list the scratch registers in use and the out-of-registers error mentions the bytes held for them.
Summary
Adds the shared infrastructure that the new PRU blocks in the following pull requests build on, fixes defects in
existing blocks, and adds a small pytest harness that generates designs with the SysConfig 1.28 CLI. Based on main
(
38266985).What changes
common/register_allocation/pru_register_allocator.js): a value's registers are released afterits last data consumer (loop-invariant values included); Loop pre-initialisation, accumulator bindings and exported
loop results; per-output-port routing, with a validation error for a connection to a port that owns no register;
a recorded error when the register pool is exhausted; reservation of registers typed into block fields, with
conflict detection; multi-output macro destinations. TI's 64-bit (lo/hi register) sources and outputs are kept.
common/pru_blocks_static_module.syscfg.js): validation of allocation failures, typed-registerconflicts, unconnected required inputs, invalid port links, loop/container boundaries and macro arity; CTPPR0 setup
(c28 to PRU shared RAM) and the SPI CS idle level at program start and at every group start; GUI warnings for
GPO/GPI pads shared between blocks and for duplicate GPO drivers (warnings only, code generation still runs); a
device-capability check for blocks that need a specific hardware widget, failing closed when the capability data is
unavailable.
pru_syscfg.asm.xdt,pru_syscfg.inc.xdt): TEMP_REG3/TEMP_REG4 (R26/R27) and the prologue.pru_reserved_names.js(clpru reserved identifiers, checked with clpru 2.3.3),pru_spi_timing.js(SPI delay limits and program-memory budget),pru_device_caps.js(per-SoC hardware widgets),block_control_flow_data.js.register.
advanced its bit counter in the low half, so SCLK ran one cycle short high and one cycle long low; MODE3 LSB took
one more cycle to store a 1 than a 0, so the high half grew by a cycle on every 1 bit. Both now count in the half
the MSB variant counts in, and both sample paths are two instructions. Both macros keep their instruction count.
an undefined
cmp_FALSEsymbol).exported results and accumulators.
getPinUsageon the GPI/GPO/SPI blocks.examples/uart_crc/firmware/main.asmand the two pru1example.syscfgfiles: see "Changes to a TI example".linker.cmdfiles (38 of the 39; the Tamagawa one already had them):.udmemis placed in the data RAM thefile uses for
.bss, and.usmeminPRU_SHAREDMEM. The Memory Variable block reserves its buffers in these twosections; with no rule clpru placed them in instruction RAM after
.text.tests/test_linker_section_rules.pychecks every tracked linker file (38 of 40 cases fail on main's files; all pass here).
tests/test_syscfg_add_instance.py(every module adds an instance through the SysConfig CLI),tests/test_fail_closed_gating.py,tests/test_shared_memvar_rename.py,tests/test_conditional_unconnected_branch.py,tests/test_uart_crc_firmware.py,tests/test_linker_section_rules.py, withscripts/*_runner.js..gitignoreaddsbuild_out/.Overlap with the open pull request "Fix if/else block issues and other issues"
That pull request touches the same areas: the allocator's
pushInstruction, the label loops inpru_syscfg.asm.xdt, and validation / documentation of the conditional, loop and delay blocks. This change does notswitch
longDescriptiontogetAIContextand does not touchai.component.jsorlabel.syscfg.js. Whichever mergessecond needs a rebase in those files.
Changes to a TI example
The generated code of
examples/uart_crcchanges register use: values are now released after their last consumer, soR0 bytes are reused. The example's
main.asmread allocator registers it did not own, so this change updates it:registers are named once, and the received frame is kept live on the receiving core.
tests/test_uart_crc_firmware.pyruns the example on both boards and both cores and checks the stored data byte and the CRC match / mismatch result.
With TI's unchanged example this test fails on this branch (6 of 7); with the update it passes (7 of 7).
How it was tested
Tools: SysConfig 1.28 (
SYSCFG_PATH), TI PRU CGT 2.3.3 (CGT_TI_PRU_PATH), and for the firmware test a checkout ofTI pru-simulator (
PRU_SIMULATOR_PATH; run here against its main branch,544d5ea).PRU_SIMULATOR_PATH: 67 passed, 0 failed, 0 skipped;PRU_SIMULATOR_PATH: 61 passed, 0 failed, 6 skipped (the six uart_crc firmware cases);PRU_SIMULATOR_PATH, gmake on PATH and a repository-rootimports.mak: 67 passed, 0 failed, 0 skipped.Every TI firmware example (39 projects under
examples/**/firmware/**/ti-pru-cgt/makefile) was built with its ownmakefile (
gmake all, SysConfig 1.28, clpru 2.3.3,DEVICEset to the project'sEXPECTED_DEVICE), which links itwith its
linker.cmd: 39 of 39 on main and 39 of 39 on this branch.SysConfig Sanity Tests: the eight contexts of
make syscfg-tests-am243x,syscfg-tests-am64xandsyscfg-tests-am261x(AM243x_ALX_beta, AM243x_ALV_beta and AM64xicss_g0_pru0/icss_g0_pru1, AM261xicss_m0_pru0/icss_m0_pru1), each run as the makefile runs it with SysConfig 1.27.0(
sanityTests.js -s .metadata/product.json -d <device> -c <core> --excludeTests="migrateToAnyTarget"): every contextreports
Total 0 error(s)and no line containing "failed", which is the workflow's pass condition.Where the fixes are tested
rename, unconnected If/Else branch, uart_crc firmware.
read/transfer zero-extend, loop accumulators, Load Constant width, register-pool exhaustion, fan-out register alias,
nested hardware LOOP, SPI CS timing, and the SPI read SCLK widths (
tests/test_spi_sclk_duty.py: every read,write and transfer variant in all four modes and both bit orders; without the SPI read fix
the MODE1 and MODE3 LSB read cases fail, 2 of 24).
(
tests/test_memory_placement.py). In the application pull request: CTPPR0 setup,reserved names. In the documentation pull request: delay cycles, SPI CS idle prologue.
Known limitations
getPinUsage, so the pad-sharing warnings do not see UART pins.R26/R27.
common/simulation/pru_core.js) in this pull request does not executeZERO; thedevice-model / simulator pull request adds it.
ldi r_dataReg, 0that clears the result register.🤖 Generated with Claude Code