Repository navigation
Size a store to a packed struct element of an unpacked array from the element's width (§7.4.1, §7.4.5, §10.4.2, §10.3, #997) - #998
Open
chuanseng-ng wants to merge 3 commits into
Conversation
… element's width (§7.4.1, §7.4.5, §10.4.2, §10.3, aionhw#997)
…4.5, §10.4.2, §10.3, aionhw#997)
…4.5, §10.4.2, §10.3, aionhw#997)
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
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.
Fixes #997.
Problem
A nonblocking store, a continuous assignment or a delayed nonblocking store to a packed struct element of an unpacked array kept one bit: on
le_t [1:0] b [2]withle_ta 40-bit packed struct,b[0][1] <= 40'hDEAD0000A5leftb[0][1] = 0000000001(§7.4.1, §7.4.5, §10.4.2, §10.3). A row of two struct elements (dd[1][0] <= {v2, v}on a[1:0][1:0]struct array) kept two bits. Blocking stores were right.Cause
The interpreter sizes the right-hand side of these stores with
infer_lhs_width(src/compiler/simulator/method_types.rs). Past the unpacked dimensions itsIndexarm multiplies the remaining dimensions ofpacked_full_dims, which lists the packed array dimensions only; a packed struct element adds none, so the product leaves out the struct's own width. A select of a whole struct element has no dimension left and falls through to one bit, and the right-hand side is resized to it before it is queued.Fix
packed_select_width_in_unpacked_elem: past the unpacked dimensions, the width is the unpacked element's declared width (arrays,arrays_2d,arrays_nd) over the number of packed elements the selects pick from (the product of the selectedpacked_full_dims, or the packed element count when only a single[N-1:0]range is recorded). It answers only when that division is exact and leaves more than one bit, so plain vector elements keep the existing path.Indexarm ofinfer_lhs_widthasks it first when the chain goes past the unpacked dimensions.Only the interpreter changes: compiled code and the JIT are untouched, so
METHOD_CACHE_COMPILER_SALTstays as it is.Tests
tests/types/packed_struct_elem_store_width.rs, 4 tests, every expected value from the reference simulator. Onc35707e63 of the 4 fail;stores_that_already_work(member and part-select stores, whole rows, a blocking store, a packed-only array of structs, a typedef vector element) passes as a guard.c35707e6always, package struct, 1-D and 2-D)a=dead0000a5 f=dead0000a5a=0000000001 f=0000000001pkgT=dead0000a5 localT=dead0000a5 typedefvec=dead0000a5 inline=dead0000a5 packedonly=dead0000a5 twoD=dead0000a5 dyn=dead0000a5pkgT=0000000001 localT=0000000001 typedefvec=dead0000a5 inline=0000000001 packedonly=dead0000a5 twoD=0000000001 dyn=0000000001u=dead0000a51234567890 o=1234567890dead0000a5 k=dead0000a50000000000 dd=0000000000dead0000a51234567890dead0000a5u=00000000010000000000 o=00000000000000000001 k=00000000010000000000 dd=0000000000000000000100000000000000000001always_ff, delayed nonblockingca=dead0000a50000000000 ff=0000000000dead0000a5 dl=dead0000a50000000000ca=00000000010000000000 ff=00000000000000000001 dl=00000000010000000000pu[0][0]is the case fixed)… pu[0][0]=dead0000a5 …… pu[0][0]=0000000001 …always_ffpush intostk[0][0]stk[0][0]=dead0000a5,PASSstk[0][0]=0000000001,FAILFAILremains (see below)c35707e6+ this branch, pinned core2ff85ad): 6614 passed, 0 failed, 1 ignored.perfis not available on this machine (perf_event_paranoid=4), so no instruction counts. The added work is one or two table lookups when a store's left-hand side is an index chain past the unpacked dimensions.Caches and notes
Note …commit adds theNOTES.mdentry under### Unreleased. Read a packed element selected inside an unpacked array's element in compiled code (§7.4.1, §7.4.5, §11.5.1, #985) #986 adds the same new### Unreleased/ Correctness heading, so whichever lands second needs a one-line merge (one heading, both bullets).The last probe's
FAILcomes from itsif (stk[0][0] === 40'hDEAD0000A5)read in compiled code, the read #985 / #986 fixes: with a blocking store instead, xezimc35707e6printsFAILtoo, and the #986 branch printsPASS.Not covered: two packed selects into a typedef-named struct array (
le_t [1:0][1:0] dt [2],dt[1][1][0] <= v) still keep one bit, because the elaborator records no packed dimensions for a typedef-named type (#984). The class-property case in #928 has a different cause.