RapidWright
RapidWright copied to clipboard
Fix #422: Correctly resolve HierObject search with prefix match and '/' in name
This is an attempt to resolve incorrect resolution of a hierarchical name search of a netlist object that happens to use a hierarchy separator (/
) in its name AND also has another instance name collision prefix in the existing netlist hierarchy. The issue is illustrated in #422.
Unfortunately, this implementation will increase runtime on those cases where a cell instance is queries but does not exist.
Unit Test Results
35 files 35 suites 9m 9s :stopwatch: 548 tests 540 :heavy_check_mark: 8 :zzz: 0 :x: 556 runs 548 :heavy_check_mark: 8 :zzz: 0 :x:
Results for commit 5ef419f4.
:recycle: This comment has been updated with latest results.
Crazy alternative idea: What if we store a cell's children in some kind of tree?
For example, to store cells a, a/b, a/c and b/c (all of these are names with slashes, not actual hierarchy):
graph TD
root --> |a| a
a -->|b| a/b
a -->|c| a/c
root[root] -->|b| B(null)
B -->|c| b/c
How many slashes do we typically see in a CellInst's name? This idea would waste memory if we have names with many slashes in them without the prefix-cells also existing.
How often do we query by relative name (made slower by this tree idea) versus by hierarchical name (improved by this)? Or would we spend the memory to have both representations?
How many slashes do we typically see in a CellInst's name? This idea would waste memory if we have names with many slashes in them without the prefix-cells also existing.
Most designs do not have any cell instances with slashes in their name. A small percentage of designs do have some cell instances with slashes, and typically, I only see one slash per name, although it is quite possible to have more.
How often do we query by relative name (made slower by this tree idea) versus by hierarchical name (improved by this)? Or would we spend the memory to have both representations?
I think this is a rather rare circumstance, I don't really want to spend much to support it. I think it will be slower when designs have this. I think the idea is interesting and has merit, but, I think a more practical approach of flagging cells that have this as a way to trigger special their handling.
Does the test here pass today?