search

byuccl / spydrnet

A flexible framework for analyzing and transforming FPGA netlists. Official repository.
https://byuccl.github.io/spydrnet
BSD 3-Clause "New" or "Revised" License
86 stars 20 forks source link

Case Sensitivity - EDIF composer #182

Open emonlux opened 2 years ago

emonlux commented 2 years ago

Unable to generate a TMR netlist ValueError: Applying name would result in a naming conflict

namespace_manager -> init.py Line 107 - if parent and parent in self.namespaces:

possible fix - if parent and element in self.namespaces:

jacobdbrown4 commented 1 year ago

I'm not super familiar with the namespace manager. However, line 107 seems like it's checking to make sure that parent is not None and that it's in self.namespaces.

emonlux commented 1 year ago

Yeah I eventually figured that out. Parent and Parent seems redundant though

jacobdbrown4 commented 1 year ago

So it looks like it's an issue with upper-case and lower-case. The namespace manager does not take case into account when checking for duplicate names. So F1_reg_TMR_0_Q_VOTER and f1_reg_TMR_0_Q_VOTER are considered the same. It's interesting that this wasn't caught when the instance was created but rather when composing the netlist.

This pull request https://github.com/byuccl/spydrnet/pull/176 will attempt to fix the issue and give an option to support case sensitivity.

jacobdbrown4 commented 1 year ago

It's also interesting that your generated netlist had the same names just different case. Where did the netlist come from?

emonlux commented 1 year ago

The netlist came from Vivado

jacobdbrown4 commented 1 year ago

Yeah I eventually figured that out. Parent and Parent seems redundant though

As for this, it may seem redundant but I don't think it is. It's checking to make sure the element has a parent, and then it's seeing if it's in self.namespaces. The writer may have done this because 'None' (which the parent would be if it didn't exist) may be in self.namespaces and may cause problems if we try to access it. So if the parent is 'None' it is passed by.

jacobdbrown4 commented 1 year ago

That pull request looks like it gives the case sensitive option to the parser but not the composer. We probably need to add to it.

jacobdbrown4 commented 1 year ago

See the case_insensitive_naming branch for some work on this.

jacobdbrown4 commented 1 year ago

Note that this is related to issue #116

jacobdbrown4 commented 8 months ago

Note: the case_insensitive_naming branch contains code to tell the EDIF parser to be case sensitive for naming. I merged this code into next release but then removed it, so github thinks this branch has nothing new but it does.