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

Improve error handling on slices of packed arrays #226

Closed
veripoolbot opened this issue Mar 19, 2010 · 8 comments
Closed

Improve error handling on slices of packed arrays #226

veripoolbot opened this issue Mar 19, 2010 · 8 comments

Comments

@veripoolbot
Copy link
Contributor

@veripoolbot veripoolbot commented Mar 19, 2010


Author Name: Byron Bradley (@bbradley)
Original Redmine Issue: 226 from https://www.veripool.org
Original Date: 2010-03-19
Original Assignee: Byron Bradley (@bbradley)


The attached patch improves error handling on slices of packed arrays:

  • We didn't throw an error if a constant value was assigned to an entire array. This could give wrong runtime results.
  • Because an assign was cloned, an error would be printed once for each clone. Only print one slice error per assign.
@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Mar 21, 2010


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2010-03-21T01:29:57Z


Thanks! A test even!

In git for 3.802+

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Mar 23, 2010


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2010-03-23T20:55:42Z


From: Lane Brooks
Subject: [Verilator-dev] array issues

I haven't been following the array/slice changes, but I recently pulled
and rebased the tristate changes I have been working on and my design is
now failing to verilate with an error:

%Error: ../../UXN1230/lib/rtl/HostInterface/models/fx2.v:46:
Unsupported: Assignment between packed arrays of different dimensions

The verilog of line in question is:

reg [15:0] wbuf[0:255] /* verilator public */;

I git reverted commit 6715cb9 and it
verilates fine as before.

Do you need me to further isolate the problem or is this sufficient? The
error message and line number are cryptic enough that I do not really
know how what the issue is. The array in question has been working
without issue prior to this.

Lane

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Mar 24, 2010


Original Redmine Comment
Author Name: Lane Brooks
Original Date: 2010-03-24T08:46:19Z


Previously I mentioned that even after reverting this patch that I was still having problems. That was not correct. So in summary, this patch causes the above mentioned error. If I remove the /* verilator public */, then the error changes to

%Error: Internal Error: ../../UXN1230/lib/rtl/HostInterface/models/fx2.v:75: ../V3Slice.cpp:178: Couldn't find VarRef under the ArraySel

Line 75 is a read from the array inside a clocked process:

If I revert this patch, everything works fine.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Mar 24, 2010


Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2010-03-24T17:01:03Z


I haven't been able to reproduce this but I have found some possible issues with the code. Some of these were found as part of #227 so I'll probably put most/all of the patches there.

Wilson, at the moment V3Slice runs on every AstNodeAssign but I don't think it should. I've skipped AstAssignAlias nodes and all of the Verilator and my regression passes. Can you think of any other assign nodes that don't make it to V3Emit?

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Mar 24, 2010


Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2010-03-24T17:05:20Z


Spoke too soon, AstAssignAlias nodes do make it to V3Emit so they do need to be expanded for slices.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Mar 24, 2010


Original Redmine Comment
Author Name: Lane Brooks
Original Date: 2010-03-24T19:36:58Z


I did a little more digging into this problem, and it seems like a false alarm. I checked out the main branch without my new tristate changes and I do not see these errors. Likewise if I leave my changes in but revert this patch, I do not see the errors. So there is a conflict between this patch and my tristate changes.

So I dug a little deeper and found that the array was being tristate expanded because it did not have an any drivers (the drivers are actually in C). I decided that I will not tristate expand public variables when they have no drivers. That resolves the conflict.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Mar 24, 2010


Original Redmine Comment
Author Name: Byron Bradley (@bbradley)
Original Date: 2010-03-24T22:42:17Z


Great, thanks Lane, that explains why I couldn't reproduce it :)

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented May 1, 2010


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2010-05-01T19:02:03Z


In 3.802

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.