Add `i0` support - tracking issue

[mortbopet]

Following the 21/09/2022 ODM, a consensus was reached on moving forward with supporting zero-width values in the core RTL dialects.

If you are aware of any location which currently performs special case logic for zero-width values and/or needs consideration during this work, please add it to the list at the end of the issue. Ensures that we don't miss it and that people don't have to spend extra time trawling through the codebase.

• [x] Allow zero-width values in HWIntegerType
  • [x] Allow zero-width indexes to singleton arrays #3985
  • [x] #3983
  • [x] Zero-width constant support
• [ ] Fix any location which currently contains special-case logic for zero-width integers
  • Related: https://github.com/llvm/circt/issues/2909
• [ ] ... fix any location which supports parametric hw int types and does not consider the possibility of resolved types being zero-width
• [x] Add pruning pass (continue #3935). Said pass should prune all of the i0 logic from inside modules, but keep i0 module ports. Summarizing the ODM, a pass-based approach is preferable due to:
  • Testability
  • Traceability - any value pruned can be replaced with an sv comment stating the former existence of that value.
  • We already have passes which are expected to be run for putting the IR in a legal state prior to emission (--hw-legalize-modules).
• [ ] https://github.com/llvm/circt/issues/4018
• [x] Include pruning pass in a predefined prepare-for-export pass pipeline alongside passes such as --hw-legalize-modules.
• [ ] Fix canonicalizer

Locations to modify:

(add stuff...).

[teqdruid]

I would actually merge the pruning logic into PrepareForEmission. That's the correct place for it, as of today and it's already a separate pass. If there's interest in breaking up PrepareForEmission in the future (for debuggability), that'll be the time to break this out.

What I'd like to avoid is having this pruning (which is required) to be different from every other required preparation step. IMO, we should either break all of them out and have a "PrepareForEmission" pipeline or we break none of them out. I don't think it's wise to do this piecemeal.

[teqdruid]

Also, the "legalize modules" pass is a bit of a misnomer as it's not strictly required. AFAICT it's only needed to support lowering options for certain tools which don't have support for the SystemVerilog we produce.

[mortbopet]

IMO, we should either break all of them out and have a "PrepareForEmission" pipeline or we break none of them out. I don't think it's wise to do this piecemeal.

I'd be in favor of breaking it up as much as possible (assuming that is possible, and the separate pieces of PrepareForEmission can actually run in isolation). Separating the concerns of each preparation step seems natural to me, and it should be easy to compose these into a predefined pipeline that we require to be run.

[uenoku]

I agree hw-legalize-module is not good place for this. I'm in flavor of separating the pass from PrepareForEmission (probably we can reuse the pass for SystemC backend as well). I think we can do the same thing as what we are doing for PrepareForEmission. Currently PrepareForEmission is not invoked from pass manager, it is called from ExportVerilog pass as a method prepareForEmission(HWModule op, ..). https://github.com/llvm/circt/blob/191e4b8ba580fc10d6fce6bd112feeb2df61cd52/lib/Conversion/ExportVerilog/ExportVerilog.cpp#L4988-L4989 PrepareForEmission has a dedicated pass test-prepare-for-emission for testing purpose. Though I think it makes sense to refactor the current ExportVerilog pass into ExportVerilog "pass pipeline" which includes ZeroValuePruning, PrepareForEmission and VerilogEmitter.

[teqdruid]

I'm in flavor of separating the pass from PrepareForEmission (probably we can reuse the pass for SystemC backend as well).

I'm in favor of breaking out PrepareForEmission into multiple passes -- I'm sure that this isn't the only necessary verilog emission logic which could be useful for systemc emission. I just don't think this one should be special. We should have one big PR which does the break up and pass pipeline creation. Before that PR, all of the necessary logic for correct verilog emission (including the pruning pass) should go into PrepareForEmission for consistency.

[mortbopet]

To land #3985, i think we need consensus on what happens when, wrt. landing i0 type support, the pruning pass, and any modifications to the VerilogEmitter. Whether we land #3985 first, land a pruning commit first (hard to test without this), or create a large monolithic commit (I'm sensing hesitancy towards doing too much in a single PR, see #3935).

To me, the following would make sense:

1. Land #3985 (i0 safety at emission time). That means that the initial (and safe) support is in tree (unblocking me) and there's hopefully less chicken-and-egg'ing around how to proceed with i0 support.
2. Figure out how and what needs to be pruned, and get this into PrepareForEmission.
3. Refactor EmitVerilog to remove all special-case // Zero Width: ... emission code - I'm expecting that the pruning pass will replace any pruned logic with SV comments (hopefully simplifying verilog emission).

[teqdruid]

This sounds like the right ordering to me.