lip6 / coriolis

Coriolis VLSI EDA Tool (LIP6)
https://coriolis.lip6.fr
GNU General Public License v2.0
44 stars 5 forks source link

naming: Never remove underscores from original name #119

Open gatecat opened 5 months ago

gatecat commented 5 months ago

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.

jpc-lip6 commented 5 months ago

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).

gatecat commented 5 months ago

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.

jpc-lip6 commented 5 months ago

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.

gatecat commented 5 months ago

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 commented 5 months ago

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

jpc-lip6 commented 5 months ago

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

jpc-lip6 commented 5 months ago

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.

gatecat commented 5 months ago

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.

jpc-lip6 commented 5 months ago

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.