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

Auto module instantiation cannot handle module with parameters correctly #159

Open
yiancar opened this issue Apr 27, 2022 · 7 comments
Open

Comments

@yiancar
Copy link

yiancar commented Apr 27, 2022

I dont know if this is relevant but I am using the forced fast indexing else the indexing never finishes. The workspace is very large.

Using v0.13 of the extension.

First lets start with a simple module:

module simple (
  input wire  logic clk,
  output wire logic a
  );

This works correctly:

simple u_simple (
    .clk        (clk),
    .a          (a)
);

However I do get in the output log:

-: <stdin>:3325:104: syntax error, rejected ")" (syntax-error).
<stdin>:3351:104: syntax error, rejected ")" (syntax-error).
<stdin>:3381:104: syntax error, rejected ")" (syntax-error).
<stdin>:3415:104: syntax error, rejected ")" (syntax-error).
<stdin>:3441:104: syntax error, rejected ")" (syntax-error).
<stdin>:3467:104: syntax error, rejected ")" (syntax-error).
<stdin>:3493:104: syntax error, rejected ")" (syntax-error).
<stdin>:3519:104: syntax error, rejected ")" (syntax-error).
<stdin>:3545:104: syntax error, rejected ")" (syntax-error).
<stdin>:3571:104: syntax error, rejected ")" (syntax-error).
<stdin>:3597:105: syntax error, rejected ")" (syntax-error).
<stdin>:3627:105: syntax error, rejected ")" (syntax-error).
<stdin>:3657:105: syntax error, rejected ")" (syntax-error).
<stdin>:3687:105: syntax error, rejected ")" (syntax-error).
<stdin>:3721:105: syntax error, rejected ")" (syntax-error).
<stdin>:3751:105: syntax error, rejected ")" (syntax-error).
<stdin>:3793:105: syntax error, rejected ")" (syntax-error).
<stdin>:3823:105: syntax error, rejected ")" (syntax-error).
<stdin>:3853:104: syntax error, rejected ")" (syntax-error).
<stdin>:3887:105: syntax error, rejected ")" (syntax-error).
<stdin>:3917:105: syntax error, rejected ")" (syntax-error).
<stdin>:3947:105: syntax error, rejected ")" (syntax-error).
<stdin>:3977:105: syntax error, rejected ")" (syntax-error).
<stdin>:4007:105: syntax error, rejected ")" (syntax-error).
<stdin>:4041:105: syntax error, rejected ")" (syntax-error).
<stdin>:4071:105: syntax error, rejected ")" (syntax-error).
<stdin>:4137:105: syntax error, rejected ")" (syntax-error).
<stdin>:4171:105: syntax error, rejected ")" (syntax-error).
<stdin>:4201:105: syntax error, rejected ")" (syntax-error).

Now if the module has a parameter:

module simple_parameters 
 #(
    parameter TEST_PARAM=10
  ) (
  input wire  logic clk,
  output wire logic a
  );

Incorrectly the hookup is:

simple u_simple (
    .10        (10)
);

and the output log is:

-: <stdin>:240:6: syntax error, rejected "10" (syntax-error).
<stdin>:3322:104: syntax error, rejected ")" (syntax-error).
<stdin>:3348:104: syntax error, rejected ")" (syntax-error).
<stdin>:3378:104: syntax error, rejected ")" (syntax-error).
<stdin>:3412:104: syntax error, rejected ")" (syntax-error).
<stdin>:3438:104: syntax error, rejected ")" (syntax-error).
<stdin>:3464:104: syntax error, rejected ")" (syntax-error).
<stdin>:3490:104: syntax error, rejected ")" (syntax-error).
<stdin>:3516:104: syntax error, rejected ")" (syntax-error).
<stdin>:3542:104: syntax error, rejected ")" (syntax-error).
<stdin>:3568:104: syntax error, rejected ")" (syntax-error).
<stdin>:3594:105: syntax error, rejected ")" (syntax-error).
<stdin>:3624:105: syntax error, rejected ")" (syntax-error).
<stdin>:3654:105: syntax error, rejected ")" (syntax-error).
<stdin>:3684:105: syntax error, rejected ")" (syntax-error).
<stdin>:3718:105: syntax error, rejected ")" (syntax-error).
<stdin>:3748:105: syntax error, rejected ")" (syntax-error).
<stdin>:3790:105: syntax error, rejected ")" (syntax-error).
<stdin>:3820:105: syntax error, rejected ")" (syntax-error).
<stdin>:3850:104: syntax error, rejected ")" (syntax-error).
<stdin>:3884:105: syntax error, rejected ")" (syntax-error).
<stdin>:3914:105: syntax error, rejected ")" (syntax-error).
<stdin>:3944:105: syntax error, rejected ")" (syntax-error).
<stdin>:3974:105: syntax error, rejected ")" (syntax-error).
<stdin>:4004:105: syntax error, rejected ")" (syntax-error).
<stdin>:4038:105: syntax error, rejected ")" (syntax-error).
<stdin>:4068:105: syntax error, rejected ")" (syntax-error).
<stdin>:4134:105: syntax error, rejected ")" (syntax-error).
<stdin>:4168:105: syntax error, rejected ")" (syntax-error).
<stdin>:4198:105: syntax error, rejected ")" (syntax-error).

If the parameter list of the module is empty #(), this produces a module instantiation without ports.

@joecrop
Copy link
Collaborator

joecrop commented May 9, 2022

Thanks for pointing this out. I have confirmed the bug and found a fix. The parameter detection is off. I'll merge the fix shortly.

@yiancar
Copy link
Author

yiancar commented May 9, 2022

Hey @joecrop thank you!
Also a suggestion for 2 enhancements:

  1. Inherit the parameters as well.
  2. Inherit any package imports.

I understand 2 is a bit of a weird one so feel free to ignore the suggestion:)
Possibly 1 could be easier to do?

Cheers,
Yiangos

@joecrop
Copy link
Collaborator

joecrop commented May 9, 2022

Can you give me examples about what you are proposing?

@yiancar
Copy link
Author

yiancar commented May 10, 2022

Hello!
Thank you for your amazing work:D
Ok the parameter passthrough is correct already :D
However I found another small bug

lets say we are trying to instantiate:

module a_with_parameters #(
    `include "design_settings.sv",
    parameter PARAM2 = 3
) (
    input  logic clk,
    output logic valid
);

The result should be:

a_with_parameters #(
    `include   "design_settings.sv",
    .PARAM2    (3)
) u_a_with_parameters (
    .clk       (clk),
    .valid     (valid)
);

However incorrectly the `include is parsed like a parameter:

a_with_parameters #(
    ."design_settings.sv" ("design_settings.sv"),
    .PARAM2               (3)
) u_a_with_parameters (
    .clk                  (clk),
    .valid                (valid)
);

I understand that not everyone might be using this includes so if it too much trouble to fix please dont worry.

Cheers,
Yiangos

@joecrop
Copy link
Collaborator

joecrop commented May 17, 2022

I agree that the result from the instantiator is definitely wrong when there is an include in the definition. But are you sure that copying that include will work? I imagine that the contents of the include would look like this:

parameter PARAM_3 = 5,
parameter PARAM_4 = 0,
parameter PARAM_5 = 40,

So if you tried to put that in the module instantiation, it would effectively look like this after compilation:

a_with_parameters #(
    parameter PARAM_3 = 5,
    parameter PARAM_4 = 0,
    parameter PARAM_5 = 40,
    .PARAM2               (3)
) u_a_with_parameters (
    .clk                  (clk),
    .valid                (valid)
);

This wouldn't compile, would it?

@yiancar
Copy link
Author

yiancar commented May 19, 2022

Hello!
Yes you are correct that would NOT work.
However if the include file had:

.PARAM_3 (PARAM_3),
.PARAM_4 (PARAM_4),
.PARAM_5 (PARAM_5),

Then it would work.

In the end its up to the user to make sure that the include files are correct in my opinion.

In multiple projects I have worked we have an include file with setting parameters and then a second one which just has parameter connections.
The setting one is used in top modules and then then connection one is used on children modules. This way we dont repeat the connection code all the time.

@angelo558
Copy link

Hi @joecrop
The fix does not seem to work for me, I have the exact same issue as described while running on v 0.13.3, any idea why?

Thanks in advance!

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

No branches or pull requests

3 participants