Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support assignment of slices of multidimensional arrays #170

Closed
veripoolbot opened this issue Nov 3, 2009 · 7 comments
Closed

Support assignment of slices of multidimensional arrays #170

veripoolbot opened this issue Nov 3, 2009 · 7 comments
Labels
resolution: fixed Closed; fixed type: feature-IEEE Request to add new feature, described in IEEE 1800

Comments

@veripoolbot
Copy link
Contributor


Author Name: Byron Bradley (@bbradley)
Original Redmine Issue: 170 from https://www.veripool.org
Original Date: 2009-11-03
Original Assignee: Byron Bradley (@bbradley)


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2009-11-03T11:16:49Z


This is a fairly easy starter project, let me know if you'd like to get started and I'll give pointers.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2009-12-05T21:58:30Z


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2009-12-06T17:11:05Z


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2009-12-14T17:49:15Z


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.

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2010-01-19T13:59:26Z


A patch to support slices is attached. There are a few unsupported features but they are detected and print a suitable error message:

  • 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]

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2010-01-19T15:58:44Z


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

@veripoolbot
Copy link
Contributor Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2010-02-07T12:37:41Z


In 3.800.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution: fixed Closed; fixed type: feature-IEEE Request to add new feature, described in IEEE 1800
Projects
None yet
Development

No branches or pull requests

1 participant