rdaly525 / coreir

BSD 3-Clause "New" or "Revised" License
100 stars 24 forks source link

Fix compatability issue with slice inline logic #952

Closed leonardt closed 4 years ago

leonardt commented 4 years ago

This fixes an issue where the insertion of mantle wires for slice inlinining was clobbering port names. Normally this isn't a problem when using --inline since the clobbered name is removed, but it's a problem for old code that doesn't use inline (e.g. garnet). This adds compatability for those flows.

Here's an example error we were getting

%Error: Interconnect.v:41356: Unsupported in C: Cell has the same name as variable: 'Tile_X01_Y01_lo'
651mantle_wire__typeBit8 Tile_X01_Y01_lo (
652                      ^~~~~~~~~~~~~~~
653        Interconnect.v:41107: ... Location of original declaration
654wire [7:0] Tile_X01_Y01_lo;
655           ^~~~~~~~~~~~~~~
656%Error: Interconnect.v:41438: Unsupported in C: Cell has the same name as variable: 'Tile_X01_Y02_lo'
657mantle_wire__typeBit8 Tile_X01_Y02_lo (
658                      ^~~~~~~~~~~~~~~
659        Interconnect.v:41145: ... Location of original declaration
660wire [7:0] Tile_X01_Y02_lo;
661           ^~~~~~~~~~~~~~~
662%Error: Interconnect.v:41538: Unsupported in C: Cell has the same name as variable: 'Tile_X02_Y01_lo'
663mantle_wire__typeBit8 Tile_X02_Y01_lo (
664                      ^~~~~~~~~~~~~~~
665        Interconnect.v:41188: ... Location of original declaration
666wire [7:0] Tile_X02_Y01_lo;
667           ^~~~~~~~~~~~~~~
668%Error: Interconnect.v:41616: Unsupported in C: Cell has the same name as variable: 'Tile_X02_Y02_lo'
669mantle_wire__typeBit8 Tile_X02_Y02_lo (
670                      ^~~~~~~~~~~~~~~
671        Interconnect.v:41223: ... Location of original declaration
672wire [7:0] Tile_X02_Y02_lo;
673           ^~~~~~~~~~~~~~~
674%Error: Interconnect.v:41347: Found definition of 'Tile_X01_Y01_lo' as a CELL but expected a variable
675    .lo(Tile_X01_Y01_lo),
676        ^~~~~~~~~~~~~~~
677%Error: Interconnect.v:41357: Found definition of 'Tile_X01_Y01_lo' as a CELL but expected a variable
678    .in(Tile_X01_Y01_lo),
679        ^~~~~~~~~~~~~~~
680%Error: Interconnect.v:41429: Found definition of 'Tile_X01_Y02_lo' as a CELL but expected a variable
681    .lo(Tile_X01_Y02_lo),
682        ^~~~~~~~~~~~~~~
683%Error: Interconnect.v:41439: Found definition of 'Tile_X01_Y02_lo' as a CELL but expected a variable
684    .in(Tile_X01_Y02_lo),
685        ^~~~~~~~~~~~~~~
686%Error: Interconnect.v:41529: Found definition of 'Tile_X02_Y01_lo' as a CELL but expected a variable
687    .lo(Tile_X02_Y01_lo),
688        ^~~~~~~~~~~~~~~
689%Error: Interconnect.v:41539: Found definition of 'Tile_X02_Y01_lo' as a CELL but expected a variable
690    .in(Tile_X02_Y01_lo),
691        ^~~~~~~~~~~~~~~
692%Error: Interconnect.v:41607: Found definition of 'Tile_X02_Y02_lo' as a CELL but expected a variable
693    .lo(Tile_X02_Y02_lo),
694        ^~~~~~~~~~~~~~~
695%Error: Interconnect.v:41617: Found definition of 'Tile_X02_Y02_lo' as a CELL but expected a variable
696    .in(Tile_X02_Y02_lo),
leonardt commented 4 years ago

Related to https://github.com/rdaly525/coreir/issues/935, I think the Verilog backend needs to keep track of used symbols and generate fresh ones to avoid this name clobbering in general. For example, if we have an instance foo, we'll generate an input port wire foo_I if needed, but there could be an instance foo_I so we need to check for conflicting names first. This is a temporary workaround to unblock downstream users, I'll work on the more general solution to avoid this name clobbering next.