Issue #170
Support assignment of slices of multidimensional arrays
| Status: | Closed | Start: | 11/03/2009 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assigned to: | Byron Bradley | % Done: | 0% |
|
| Category: | Unsupported | |||
| Target version: | - |
Description
Support assignment of slices of multidimensional arrays, for example: logic [31:0] data_pipe[255:0]; data_pipe[255:1] <= data_pipe[254:0]; Testcase will be attached soon.
History
Updated by Wilson Snyder 10 months ago
- Category set to Unsupported
- Status changed from New to Feature
This is a fairly easy starter project, let me know if you'd like to get started and I'll give pointers.
Updated by Wilson Snyder 9 months ago
- Assigned to set to Byron Bradley
First, there's the parsing. [bit] and [vector_select] are the same in the early stages so it might just work as-is. V3WidthSel is where it converts the parsed [] into AstArraySel (array extraction) and AstSel (bit selection). At present AstArraySel can only choose a single element. Modify this, or maybe better make an AstSliceSel to represent the slice. (Rummage... AstArraySel isn't used much, so I think you're better off modifying ArraySel to take a start and length similar to AstSel. Your first change can be to add the ArraySel length and throw an assert anywhere ArraySel is used with length!=1. Then fix them one by one as they pop up.)
Then to get rid of the slice... There's non-blocking and blocking.
Blocking assignments it's just an unroll. There's two schemes, using a temporary array so you don't need to worry about ordering, or being careful to choose the proper assignment direction, which may need to be dynamic if the LHS and RHS are the same variable to avoid temps
a[x+:n] = b[y+:n]
becomes:
if (x>y)
a[x+0] = b[y+0]
a[x+1] = b[y+1]
...
else
a[x+n-1] = b[y+n-1]
...
a[x+0] = b[y+0]
Skip the IF if a & b are different variables. If x/y are constants normal constant propagation will simplify the IF for you. Also you probably need an IF to check going past the end of the array.
That seems somewhat ugly. Maybe if a&b are the same variable don't bother with the above and just use a temporary. I definately think it's worth skipping temporaries in the common case of constants though, as arrays can be large.
You'll need to do whatever algorithm recusively bottom-up in case it's something like a[x+:n][y+:o][z+:p] (a good test - also test the above IF with a loop that randomly chooses x&y at run time).
For <= assignments the complicated assignment could be moved to the post-update phase for efficiency, however I think you could just do the above as-is and it will work. Note for <= you never need to worry about temporaries, they'll get created in V3Delayed if need be.
For <= assignments the complicated assignment could be moved to the post-update phase for efficiency, however I think you could just do the above as-is and it will work. Note for <= you never need to worry about temporaries, they'll get created in V3Delayed if need be.
Where to do all this... It has to be after V3Inst (assuming you want to support arrayed port connects) and V3Delayed. It's probably logical to put it after V3Unroll in case the unroller can constify the index.
It's probably obvious but do this on another checkout so the changes can be patches separately.
Thanks again for your work on this and the interfaces.
Updated by Wilson Snyder 9 months ago
It occurs to me that the IF scheme goes O(n^2) with the number of dimensions; so go with the temp's when needed.
Updated by Byron Bradley 9 months ago
Thanks Wilson, that was really helpful. I have this working for slices where the range is constant (i.e. [a:b], a, b constant) and the [x+:a] format shouldn't be too much of a problem.
I'm also handling the case where you can assign one array directly to another, i.e.:logic use_AnB; logic [1:0] active_command2 [7:0]; logic [1:0] command_A2 [7:0]; logic [1:0] command_B2 [7:0]; assign active_command = (use_AnB) ? command_A : command_B;This case is a bit more complicated because of the AstCond node under the assign. The problem I'm looking at now is in the following:
logic use_AnB; logic [1:0] active_command3 [1:0][2:0][3:0]; logic [1:0] command_A3 [1:0][2:0][3:0]; logic [1:0] command_B3 [1:0][2:0][3:0]; assign active_command3[1:0][2:0][3:0] = (use_AnB) ? command_A3[1:0][2:0][3:0] : command_B3[1:0][2:0][3:0]; (1) assign active_command3 = (use_AnB) ? command_A3 : command_B3; (2)VCS doesn't build with (1) but does with (2), but to me it feels like (1) should work. At the moment it works in Verilator with my changes but if it shouldn't then I need to detect it and error out.
Updated by Byron Bradley 8 months ago
- File slices.diff added
- Slices on non-delayed assignments where the LHS and RHS variables are the same are unsupported
- Only constants are supported in the slices
- You cannot assign between packed arrays of different lengths, e.g. [7:0][3:0] and [31:0]
Updated by Wilson Snyder 8 months ago
- Status changed from Feature to Resolved
Great stuff!!
I only have a few trivial changes; very little considering a whole new stage!
- Instead of UASSERT, use "if (xxx) nodep->v3fatalSrc()" so that the error will mention the nodep's verilog line number causing the problem. This helps users that hit it know what they can attempt to change to work around and/or report the problem.
- I added debug() methods to V3Slice, so you can "--debugi-V3Slice 9".
- Fixed a couple unused variable and other compiler warnings. (GCC 4.3.2)
- Renamed m_assign and m_lhspVarRefp to m_assignp and m_lhsVarRefp; the "p" suffix means pointer. (I see you generally seemed to follow this - thanks, so guess these two just got missed.)
- Few minor spacing issues. I also generally omit the std::.
They were minor enough, and no logical errors jumped out at me, so merged it into git mainline.
Thanks again
![[logo]](/img/veripool_small.png)