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

too many bindings specified for parameter overrides in instance #299

Open
pbreuer opened this issue Dec 14, 2024 · 7 comments
Open

too many bindings specified for parameter overrides in instance #299

pbreuer opened this issue Dec 14, 2024 · 7 comments

Comments

@pbreuer
Copy link

pbreuer commented Dec 14, 2024

I'm getting the message "sv2v: too many bindings specified for parameter overrides in instance "c" of "controller"
CallStack (from HasCallStack):
error, called at src/Convert/ResolveBindings.hs:110:9 in main:Convert.ResolveBindings" coming from line 110, just as the label on the tin says:

108 resolveBindings location available bindings@(("", _) : _) =
109    if length available < length bindings then
110        error $ "too many bindings specified for " ++ location

There is only one parameter in the sv module instance in question, but a whole pile of in/out arguments, so I imagine the size ("length") of list "available" has been exceeded and it is either a fixed length list or one the size of which is calculated dynamically based on a prior parse, BUT ... I can't find where that is, if it is so. I've looked.

Maybe there's some ghc option I don't know that traces an argument back to its origin! If so, let me know! I just use sight.

The actual in/out argument count in the instance of that "controller" module is .... 72. Exactly. Does it ring a bell? The arguments exactly match in the instance and the definition. I've checked programatically, both number and order.

(I didn't write that SV code)

Can you let me know if there is something to twiddle somewhere?

Regards

PTB

PS. Excellent job on the SV parser at the very least! I am really impressed. Accurate syntax error detection and about the only thing I notice is that it doesn't like modules where the first word of code is "begin" (thinks it should match to endmodule, not end), and there is also some foul up I haven't resolved sometimes about where the decalarations should start. That's in very old C-style formats, i.e. no use of params and commas for argument lists, just semicolons, but it hasn't gone away when I changes to the less ambiguous modern format so I don't yet know what's up there.

PPS. Ah yes, the parser doesn't seem to grok the word "ref" in a declarations list, at least sometimes. Also it doesn't seem to recognize declaration syntax "logic [a:b] array [logic [c:d]];" and no, I don't know if that's legal SV, but somebody (the authors of the code) think so ... as well as presumably knowing what it means!

@zachjs
Copy link
Owner

zachjs commented Dec 15, 2024

Thank you for reporting these issues!

I'm getting the message "sv2v: too many bindings specified for parameter overrides in instance "c" of "controller" CallStack (from HasCallStack): error, called at src/Convert/ResolveBindings.hs:110:9 in main:Convert.ResolveBindings" coming from line 110, just as the label on the tin says:

108 resolveBindings location available bindings@(("", _) : _) =
109    if length available < length bindings then
110        error $ "too many bindings specified for " ++ location

There is only one parameter in the sv module instance in question, but a whole pile of in/out arguments, so I imagine the size ("length") of list "available" has been exceeded and it is either a fixed length list or one the size of which is calculated dynamically based on a prior parse, BUT ... I can't find where that is, if it is so. I've looked.

I've just pushed 576a804 to make that error message a bit clearer. It's possible sv2v is not resolving the parameters correctly, or that there is a semantic error in the design. Please let me know what you find!

For example, one of sv2v's error checking test cases previously output:
sv2v: too many bindings specified for parameter overrides in instance "e" of "example"
but now outputs:
sv2v: too many bindings specified for parameter overrides in instance "e" of module "example": 3 specified (1, 2, 3), but only 2 available ("P", "Q")
which can hopefully narrow things down.

The actual in/out argument count in the instance of that "controller" module is .... 200. Exactly. A suspicicious number! Does it ring a bell?

sv2v doesn't impose any arbitrary limits like that.

it doesn't like modules where the first word of code is "begin" (thinks it should match to endmodule, not end)

I think the error message generated by the parser here is misleading. I don't think begin can appear as the first token in a module declaration. Please see the definition of module_common_item in IEEE 1800-2023 Section 23.2.4. Is there a version of the standard that allows this?

That's in very old C-style formats, i.e. no use of params and commas for argument lists, just semicolons, but it hasn't gone away when I changes to the less ambiguous modern format so I don't yet know what's up there.

Sorry, I'm not sure what you're referring to here. Do you have an example you can share with me?

the parser doesn't seem to grok the word "ref" in a declarations list, at least sometimes

sv2v doesn't support ref. Support could plausibly be added, but a usage example would definitely help!

logic [a:b] array [logic [c:d]];

These are called associative arrays, and I think they're not synthesizable in general, though that specific form might be. What's the use case here?

@pbreuer
Copy link
Author

pbreuer commented Dec 15, 2024

Thank you for the reply and the update, I'll see what the error message says now. The patch will also clue me as to how to squueze more out myself if needed, hopefully ...
For the rest, not in order, logic [a:b] array [logic [a:b]] appears in the testbench part of the code, not the synthesisable part; it might be good to output a specific message when ref is seen (I rewrote without ref); if a begin/end block is not allowed as the first statement in a module that is alright with me, perhaps the error message might mention it; I meant only that the declaration is "module controller import cvw::*; #(parameter cvw_t P) (input logic clk, reset, ...)" and the instance is "controller #(P) c( .clk, .reset, ...)" with all ports matching exactly, in order, between declaration and instance. The error message says "too many bindings for parameter overrides" and if parameters and not ports are meant there is only one so "too many" seems problematic.
I'll try the patch ...

@pbreuer
Copy link
Author

pbreuer commented Dec 15, 2024

It's complaining about the (1) parameter, not the (72) ports:

sv2v: too many bindings specified for parameter overrides in instance "c" of module "controller": 1 specified (P), but only 0 available

To make the patch compile I replaced "instance BindingArg TypeOrExpr where .. " with "instance (Show t,Show e) => BindingArg (Either t e) where ..". A local LANGUAGE option to permit aliases as instances would be less strong.

The declaration of the SV module is "module controller import cvw::*; #(parameter cvw_t P) ( input logic clk, reset, ...)" and the instance is "controller #(P) c( .clk, .reset, ...)".

Apparently sv2v is not picking up the parameter declaration in the controller module definition. That precise style of declaration is used in many other places. I wonder if the cvw stuff defining the type of P is not found ... it must be, all the .sv sources are expliclity given in the argument list using a 'find . *.sv'. Can I perhaps get the name of the file being processed printed out as it is processed? That would reassure me as to what is read.

Thank you for your help!

PTB

@pbreuer
Copy link
Author

pbreuer commented Dec 15, 2024

The issue has now disappeared after I played around with the source code. I suspect there were two definitions of "controller", one with a parameter and one without. I tightened the set of source files that were scanned (among other changes), and if my suspicion is correct, then that eliminated the competing definition.
I think a list of files read and in what order is needed on output, plus a warning on double definitions if that happnes (as I suspect ..).
I'm afraid this source code is not mine and has 437 files/modules and about 50,000 lines, so I really don't know a lot about what is in it (it's the wally RISCV source).

@zachjs
Copy link
Owner

zachjs commented Dec 15, 2024

The issue has now disappeared after I played around with the source code. I suspect there were two definitions of "controller", one with a parameter and one without. I tightened the set of source files that were scanned (among other changes), and if my suspicion is correct, then that eliminated the competing definition. I think a list of files read and in what order is needed on output, plus a warning on double definitions if that happnes (as I suspect ..). I'm afraid this source code is not mine and has 437 files/modules and about 50,000 lines, so I really don't know a lot about what is in it (it's the wally RISCV source).

To at least partially address this, I've added a warning for duplicate module/interface definitions in aa0a885.

@pbreuer
Copy link
Author

pbreuer commented Dec 16, 2024

Thank you very much. I have applied the patch and will run with that from now on.
I do think more positive feedback on successful processing of a file would be welcome, perhaps with -v.
I would also be very interested (desperate!) if you could report port size or type mismatches between a module instance and the module declaration. I don't know if it is allowed in SV, but I do not want that to happen!
It wouldn't be the first time a utility has a second use :). You should have enough information in there already to spot some mismatches immediately, which would be a great start.
Or do you think I could analyse the generated verilog for that?

@pbreuer
Copy link
Author

pbreuer commented Dec 29, 2024

Just a note to say that parsing dynamic arrays seems to be a necessity for translating all testbench codes that I have come across so far. Every one of them is written without any intention of being synthesizable, they are instead for testing the actual verilog that is intended for synthesis. There is no way of testing without it :-(.

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

2 participants