odb/cdl: Handle top-level pins wired to nets of different name #6040
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
clang-tidy review says "All clean, LGTM! 👍" |
smunaut
force-pushed
the
fix-cdl-pin-net-name
branch
from
f5abb2d
to
5c44c3c
Compare
|
Please add a unit test that demonstrates this use case. |
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
|
I don't see a full test, just
If no ok needs updating it means this change has no effect on any test. |
|
@maliberty Yes, I changed the input file so that it now triggers the bad behavior. With the new input file and without the code change the resulting CDL would be invalid and not match the |
maliberty
approved these changes
Nov 19, 2024
maliberty
reviewed
Nov 19, 2024
src/odb/src/cdl/cdl.cpp
Outdated
| auto pin_name_match = net2pin.find(net_name); | ||
| if (pin_name_match != net2pin.end()) { | ||
| return pin_name_match->second; | ||
| } |
There was a problem hiding this comment.
Rather than build net2pin and lookup, you could just handle the bterm here, eg
dbBTerm* bterm = net->get1stBTerm();
if (bterm) {
return bterm->getName();
}
return net->getName();
ATM the code seem to assume that for each top level pin, the net name is equal to the pin name. That's not always the case and this produces an incorect netlist. To solve that, if a net is connected to a pin, we use the pin name instead of the DB net name. Signed-off-by: Sylvain Munaut <[email protected]>
Without the previous patch fixing CDL output the tests dump_netlists / dump_netlists_withfill and the output CDL is invalid. With the path applied, the internal net name is ignored and the pin name is used instead. Signed-off-by: Sylvain Munaut <[email protected]>
smunaut
force-pushed
the
fix-cdl-pin-net-name
branch
from
50c8919
to
415f007
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ATM the code seem to assume that for each top level pin, the net name is equal to the pin name.
That's not always the case and this produces an incorect netlist.
To solve that, during pin name enumeration when creating the subckt line, we create a mapping from "net name" -> "pin name" for each pin connected to a net of a different name. Then when outputting the connections between sub blocks, we use the pin name instead of the DB net name.