general fixes - #224
Draft
DanielKellerM wants to merge 6 commits into
Draft
general fixes#224DanielKellerM wants to merge 6 commits into
DanielKellerM wants to merge 6 commits into
Conversation
The legalizer already folds the bursting decision to a literal when a port carries a single protocol class, but it still emitted a runtime protocol test to decide whether to decouple. On a port where every protocol forces decoupling the test is constant, so five of the nine backend variants shipped dead logic, and rw_obi evaluated a condition that can never be false. The test also mattered when it should not have. Software that leaves dst_protocol at its reset value gets AXI, so on an OBI-only write port the condition read false and the legalizer ran coupled and non-bursting while every other protocol-dependent decision had already folded to OBI. MAGIA hit this as a throughput collapse and worked around it in software. AXI is the only protocol with bursts != not_supported and legalizer_force_decouple false, so a port folds exactly when AXI is not one of its protocols. Drop the coupled branch, its page-boundary comparator and the now unused c_num_bytes_to_pb for those variants.
The distributed midend rebases each slice onto its region by subtracting the transfer start address, but it subtracted only the low DmaRegionAddressBits of it. For a transfer whose start lies outside region zero that drops the region index, so every slice after the first was rebased to the wrong counterpart address. Only the two address expressions are affected; the length terms subtract a within-region residue and stay truncated. MemPool hit this and fixed it downstream in mempool 1b4dc47b on 2024-01-25, but the fix was never sent back upstream.
The multi-processor midends were pickled and documented but never simulated, which is why the distributed midend shipped a wrong counterpart address for two years. Check the split geometry directly: the distributed side must carry the region-local address and the counterpart side the rebased one, for transfers that start in a non-zero region, that start in region zero, that are driven from either side, that fit in one region, and that span every back-end. The split is combinational, so the testbench never handshakes and the stream fork stays idle. Expectations derive from the parameters, so the topology sweeps without a golden table.
IDMA_TYPEDEF_REQ_T has taken a user type since the Snitch multicast work, but IDMA_TYPEDEF_FULL_REQ_T never forwarded one, so every user of the convenience macro got the logic default and a one bit user field no matter how wide the request was meant to be. That covers nearly all of the testbenches, which is why the field has never been driven or checked anywhere. Forward an optional user type. All existing call sites omit it and keep the one bit field, so nothing changes for them. The distributed midend testbench opts in with a 36 bit tag and checks it survives onto every slice.
NumDim was a localparam fixed at two, so the ND midend behind the inst64 frontend could never be given more dimensions. idma_transpose_midend asserts NumDim >= 4, which put it out of reach of this frontend entirely. Promote it to a parameter and keep the default at two, so existing instantiations are unaffected and a system that wants the transpose midend or a rank three transfer can ask for it.
An interconnect that resolves collectives in the network joins flits by destination and mask, so participants that decompose a transfer differently never join and the network stalls. The legalizer splits on the page boundary and the source offset, so matching lengths alone do not guarantee matching burst counts. Say so where DMUSER is documented, and record the new NumDim parameter in the same table.
DanielKellerM
changed the title
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.
No description provided.