RapidWright icon indicating copy to clipboard operation
RapidWright copied to clipboard

Fix #422: Correctly resolve HierObject search with prefix match and '/' in name

Open clavin-xlnx opened this issue 2 years ago • 3 comments

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.

clavin-xlnx avatar May 16 '22 22:05 clavin-xlnx

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.

github-actions[bot] avatar May 16 '22 22:05 github-actions[bot]

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?

jakobwenzel avatar May 20 '22 14:05 jakobwenzel

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.

clavin-xlnx avatar May 20 '22 21:05 clavin-xlnx

Does the test here pass today?

eddieh-xlnx avatar Sep 15 '22 23:09 eddieh-xlnx