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

Jump to bottom

problem with connecting a slice of a struct array to a port #880

Closed
veripoolbot opened this issue Feb 6, 2015 · 5 comments
Closed

problem with connecting a slice of a struct array to a port #880

veripoolbot opened this issue Feb 6, 2015 · 5 comments
Labels
area: lint Issue involves SystemVerilog lint checking resolution: fixed Closed; fixed

Comments

@veripoolbot
Copy link
Contributor

Author Name: Varun Koyyalagunta
Original Redmine Issue: 880 from https://www.veripool.org
Original Date: 2015-02-06
Original Assignee: Varun Koyyalagunta

In this example -

typedef struct packed {
  logic  p;
} s_data;

module m1 (output s_data data[1:0]);
  assign data[0].p = 0;
  assign data[1].p = 0;
endmodule

module top (output s_data data[2:0]);
`ifdef NO_SLICE
  s_data subdata[1:0];
  assign data[1:0] = subdata;
  m1 m1_inst (.data(subdata));
`else
  m1 m1_inst (.data(data[1:0]));
`endif
endmodule

verilator --cc errors with -

%Error: struct.sv:16: Illegal output port connection 'data', mismatch between port which is not an array, and expression which is an array.

verilatator --cc +define+NO_SLICE passes though.

I can help contribute a patch if the fix is tractable and someone can point me in the right direction.
@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Jie Xu (@jiexu)
Original Date: 2015-02-06T13:41:33Z

Did some investigation here. The current error comes from V3Width when Verilator try to handle V3AstPin. The issue is Verilator currently only check the if both the port and expression are arrays. In your case, the @DaTa[1:0]@ is actually an ArraySel which is selecting some part from an array. IMHO we should actually check the width match instead of simply comparing the type.

Did some other experiments:

  1. if change @module top (output s_data data[2:0]);@ to @module top (output s_data data[1:0]);@, the it will pass.
  2. if change @m1 m1_inst (.data(data[1:0]));@ to @m1 m1_inst (.data(data[2:0]));@, there will be error but it actually comes from V3Slices.

It seems to me actually it is kind of safe to disable this error since V3Slice is checking that anyway.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-02-06T15:13:14Z

Width mismatches in V3Width cannot be simply disabled, because V3Width will correct a width mismatch by padding or bit extaction, and so V3Slice will never see it. However disabling it in the specific case that the total widths match but the array depths are different seems reasonable, as then we know V3Slice will get invoked.

Would be great to have a patch for that; please include a new test_regress/t style test too. Thanks!

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Varun Koyyalagunta
Original Date: 2015-02-06T21:45:14Z

Here's a patch attempt. Contains a naive fix and a test_regress test. Thanks.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-02-11T01:25:44Z

Thanks for the patch & test. Pushed to git towards 3.869.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2015-02-13T01:41:31Z

In 3.870.

@veripoolbot veripoolbot added area: lint Issue involves SystemVerilog lint checking resolution: fixed Closed; fixed labels Dec 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: lint Issue involves SystemVerilog lint checking resolution: fixed Closed; fixed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@veripoolbot