coriolis icon indicating copy to clipboard operation
coriolis copied to clipboard

naming: Never remove underscores from original name

Open gatecat opened this issue 1 year ago • 9 comments

The rules to prevent duplicate or leading/trailing underscores make sense when other characters are being translated to underscores, but in my opinion make less sense when underscores are present in the original cell name.

For example, the gf180 cell gf180mcu_fd_ip_sram__sram512x8m8wm1 would be renamed to gf180mcu_fd_ip_sram_sram512x8m8wm1 by the code as-is, which would cause problems for GDS patching or timing analysis.

If you prefer a more conservative change that just preserves double (or more underscores) in names, I can also take that approach.

gatecat avatar Jan 29 '24 10:01 gatecat

Yes I noticed this annoying management of underscore. I agree with the principle that we should modify as little as possible the original names. My only concern is about VHDL. We must check that it's still valid for it (if not, move the renaming at this stage).

jpc-lip6 avatar Jan 29 '24 11:01 jpc-lip6

Yes, my bad, I didn't realise that VHDL apparently prohibits two consecutive underscores.

So, if you wanted to use this SRAM with VHDL, you'd have to use an extended identifier in the backend.

gatecat avatar Jan 29 '24 11:01 gatecat

I confirm, VHDL identifiers does not allow two consecutive underscore, and not trailing one either.

So the translation should be moved at VHDL driver level. If we want to preserve the original names, we also need to directly load the BLIF file and not go through the VHDL translation in between.

jpc-lip6 avatar Jan 29 '24 11:01 jpc-lip6

I'm a bit concerned about taking names from the BLIF file as-is, because a name might have a . in it (and indeed, this is quite likely when flattening with Yosys) and if that wasn't removed or escaped at the BLIF parse stage, then it would create an ambiguity with a hierarchical name further down the line.

One option for now would be to remove everything but double underscores at the BLIF parse stage and escape those specifically during VHDL export?

gatecat avatar Jan 29 '24 14:01 gatecat

@jpc-lip6 sorry for the ping, just wondering if you've had a chance to think about my last message here?

gatecat avatar Feb 02 '24 08:02 gatecat

Hello Myrtle, sorry, not yet. Lot of things to do for my lectures last week! Will try as soon as possible.

jpc-lip6 avatar Feb 02 '24 10:02 jpc-lip6

Hello Myrtle,

I may have time to look into the problem in the next days. But my first question is the "specification". If we run straight through Coriolis from BLIF to GDS, we should not have any naming problems. They should not be affected by . (dot) in names, it may only mislead a human reader looking at the layout.

On the other hand, the VHDL files generated will not match what has been processed through Coriolis and may cause discrepencies if we restart and load the VHDL netlist instead of the BLIF one.

We can also do what you suggest, that is "sanitize" the names coming from BLIF. I fail to find a description of valid blif identifiers, but is seems they encompass Verilog. So we can add an option if the BLIF parser to allow the user to request it.

Last question: I'm not sure I understand what you do mean by escaping the double underscores for the VHDL export.

jpc-lip6 avatar Feb 05 '24 21:02 jpc-lip6

If we run straight through Coriolis from BLIF to GDS, we should not have any naming problems. They should not be affected by . (dot) in names, it may only mislead a human reader looking at the layout.

This is a lot less of a concern for cell names, where it's unlikely anyway, if we get rid of VHDL cleansing in general, you could (and with Yosys flattening, almost certainly will) have a net name with a . in it, say foo.bar.

If you then want to refer to that net, say to create an HTree on it it's ambiguous between a net called foo.bar in the top level or a net called bar in an instance called foo.

Last question: I'm not sure I understand what you do mean by escaping the double underscores for the VHDL export.

Instead of replacing the double underscores with a single one, we could theoretically keep the names intact by using a VHDL extended identifier, so foo__bar becomes \foo__bar\ in VHDL, as this removes the normal rules in place for VHDL names.

gatecat avatar Feb 06 '24 13:02 gatecat

If we run straight through Coriolis from BLIF to GDS, we should not have any naming problems. They should not be affected by . (dot) in names, it may only mislead a human reader looking at the layout.

This is a lot less of a concern for cell names, where it's unlikely anyway, if we get rid of VHDL cleansing in general, you could (and with Yosys flattening, almost certainly will) have a net name with a . in it, say foo.bar.

If you then want to refer to that net, say to create an HTree on it it's ambiguous between a net called foo.bar in the top level or a net called bar in an instance called foo.

In the case of the HTree, the net must be in the core or corona. It cannot be a deep net. However I see your point, but in which case do we want to refer to a deep net through it's name ? If need be, what we would build should be a Hurricane::Occurrence which is a pair of Hurricane::Path and any Entity, in our specific case, a Net. The textual representation of the Occurrence may be the full path name. I would try, as much as possible on not relying on naming conventions for complex access. Agreed, there are some places I do it...

Last question: I'm not sure I understand what you do mean by escaping the double underscores for the VHDL export.

Instead of replacing the double underscores with a single one, we could theoretically keep the names intact by using a VHDL extended identifier, so foo__bar becomes \foo__bar\ in VHDL, as this removes the normal rules in place for VHDL names.

Ha OK. I understand and would agree if there wasn't a catch. The point of using VHDL (an, in fact, VST) is to be backward compatible with Alliance for the purpose of checking. VST (for [V]HDL [st]ructural) do not support extended identifiers, so I think there is no point in doing it. If I recall correctly, Serge did develop a Verilog parser driver, we may use it for that.

All in all, I thing this is less of a technical issue than a policy of design flow one. We should clearly define how the different part of the flow interact, choose (impose) on the users and be public about it.

It is a recurrent problem in design flows, and I think we should have clear guidelines before patching here and there.

jpc-lip6 avatar Feb 07 '24 22:02 jpc-lip6