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

AUTOINST A->B->A parameter misreplacement #1242

Closed
veripoolbot opened this issue Nov 14, 2017 · 3 comments
Closed

AUTOINST A->B->A parameter misreplacement #1242

veripoolbot opened this issue Nov 14, 2017 · 3 comments

Comments

@veripoolbot
Copy link
Collaborator


Author Name: Clarke Watson
Original Redmine Issue: 1242 from https://www.veripool.org


Hi,

I am encountering an issue where AUTOINST is declaring a vector with incorrect array bounds.

I have attached two files to illustrate the problem:

  • test_submodule.sv (this is the module being AUTOINST'ed)
  • test_top.sv (this is the top level design instantiating the submodule)

The issue is on line 57 of test_top.sv.

Here is what is produced by expanding AUTOs:
.b_tdata (a_tdata[B_WL-1:0]), // Templated

Here is what I expect to be produced based on the AUTO_TEMPLATE (note the array bounds of a_tdata):
.b_tdata (a_tdata[A_WL-1:0]), // Templated

I suspect the issue is triggered because test_submodule has a ports named a_tdata and b_tdata, but the AUTO_TEMPLATE is swapping them so port a_tdata is connected to signal b_tdata and port b_tdata is connected to signal a_tdata.

I tried the most recent version of verilog_mode.el (Nov 13, 2017).

Please take a look.

Thanks!
Clarke

@veripoolbot
Copy link
Collaborator Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-11-15T00:47:54Z


The problem as you probably guessed is you're changing B_WL->A_WL, and another rule goes A_WL->B_WL.

So the code starts with your signal t_data[B_WL], then applies the first parameter change so has now t_data[A_WL] (properly), it then keeps going to the next parameter change and goes to t_data[B_WL] (incorrectly}.

You might think I could just stop on the first replacement, but then a case like "t_data[X_WL][Y_WL]" would not work with X_WL & Y_WL (non-swapped) being replaced.

I need to think if there's an easy way to do this, short of tokenizing the entire string and looking up every single token for replacements which would be a lot of code and slow.

@veripoolbot
Copy link
Collaborator Author


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2017-11-21T23:25:50Z


So I looked at how to fix this again and prototyped a fix. Any fix for this breaks a more common convention that works now, e.g.

parameter A_WL;

then in the module:

parameter B_WL = $clog2(A_WL);

for more details see #� and autoinst_clog2_bug522.v in git.

Basically you do want Verilog-mode to follow the A_WL/B_WL replacements in order for the $clog2 to work properly. Sorry.

@veripoolbot
Copy link
Collaborator Author


Original Redmine Comment
Author Name: Clarke Watson
Original Date: 2017-11-21T23:48:47Z


Hi Wilson,

Thanks for looking into a fix.

I have a workaround: declare the array bounds in the AUTOINST template instead of letting the AUTO expansion infer the array bounds, e.g.

 .b_tdata (a_tdata[A_WL-1:0]),

Thanks,
Clarke

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

No branches or pull requests

1 participant