[HW] Add PruneZeroValuedLogic pass

[mortbopet]

Work on #2219 (#2909 related), and motivated by HandshakeToHW wanting to delegate pruning of all data-less logic to after the lowering phase, instead of having to sprinkle special-case logic all over the conversion pass itself.

This is not the support that is immediately needed for #2219, which concerns itself mostly with values that index into memories (singleton memories cannot be indexed => indexing wire is i0) but I'd expect something like this to inevitably be needed in the long run regardless.

Instead, this commit concerns itself with combinational and sequential logic which uses i0-typed values. This commit takes an aggressive approach and prunes all arithmetic (and seq.compreg) operations which uses i0 values under the assumption that any arithmetic taking part in an i0 chain will not materialize to anything in hardware.

Follow-up work:

• Switch ESI and Handshake usage of none-typed values to i0 typed
• Introduce hw.integer : i0 constant
• Erase %0 = esi.none : i0 none-typed SSA token generating op.

[mikeurbach]

Thanks for pushing on 0-width support. I think we all want to see #2219 happen, and should discuss that. But I'm not sure about the approach in this draft.

It's making a change to isHWIntegerType, which I'd argue is pretty much the center of CIRCT's core. For example, admitting parameterized types in that method implicitly broke assumptions throughout our codebase, but this wasn't handled, and the latent issue manifested when upstream MLIR got tighter about the types of Values: https://github.com/llvm/circt/pull/3859. I'd be very careful about making changes here.

The angle of this PR seems to be that you'd rely on the PruneZeroValuedLogic pass to sweep away 0-width signals, and avoid any places that assumed there are none. But I don't think it is scalable to require a specific pass for 0-width signals to be used correctly. What if someone doesn't want to run that pass, or wants a different behavior? If we are changing the core of HW's type system, I don't think it's acceptable to require a new pass for everything else to work with the changed type system.

I think we should approach this holistically, probably with an ODM discussion, as @teqdruid just mentioned on #2219. Start with the change to isHWIntegerType, and figure out all the places that relied on the non-zero width assumption. It will be painful, but we should agree on how those existing places should be updated to handle zero width signals first. Then, if you want different behavior, like PruneZeroValuedLogic, we can add such a pass. But I don't think we start by relying on a new pass to handle zero width signals in a specific way.

[uenoku]

Thank you for working on 0bit support! I agree with Mike about the concern and I'm wondering we can handle 0bit width in the very late pipeline such as PrepareForExpression/ExportVerilog. Specifically we can eliminate most of all 0bit width values in PreparetForEmission and ExportVerilog can omit remaining 0bit values (currently ExportVerilog is implemented as such).

Also we have to clearly define the semantics of comb operations with 0 bit values. I guess there is some weirdness regarding sign extension.

[mortbopet]

Seeing as the consensus yesterday fell on having some form of pass that prunes i0 values, I'll keep this PR up as a draft for now. Summarizing, a pass-based approach is preferable due to:

1. Testability
2. Traceability - any value pruned can be replaced with an sv comment stating the former existence of that value.
3. We already have passes which are expected to be run for putting the IR in a legal state prior to emission (--hw-legalize-modules).

However, before this lands, and as @mikeurbach suggests, i0 support should be added to HWInteger, existing edge cases in the code base should

[mortbopet]

@uenoku @teqdruid reworked this PR - please see the updated commit message.

• Pruning is no longer a pass but called during PrepareForEmission
• Pruning only targets SV assignment ops, for now
• Pruning doesn't erase anything but instead just marks an op with a pruning reason. This is then a signal to the emitter that an op should be commented out. This allows for good traceability (letting the emitter export verilog for the pruned logic) as well as provide a structcured way of signaling to the emitter that a given operation should be commented out. This method should be able to be leveraged to rewrite most, if not all, of the zero width emission logic currently in ExportVerilog.

[uenoku]

Thanks! I'll take a look at the details later but there are several counterexamples right now that need to be fixed:

hw.module @Concat(%arg0: i0, %clk: i1) -> (out: i1) {
  %1 = comb.concat %arg0, %clk : i0, i1
  hw.output %1 : i1
}

hw.module @icmp(%a: i0) -> (y: i1) {
  %0 = comb.icmp eq %a, %a : i0
  // And other predicates
  hw.output %0 : i1
}

hw.module @parity(%arg0: i0, %clk: i1) -> (out: i1) {
  %0 = comb.parity %arg0 : i0
  // %0 should be canonicalized into 0
  hw.output %0 : i1
}

Current outputs:

module Concat(  // foo.mlir:1:1
  // input  /*Zero Width*/ arg0,
     input                 clk,
     output                out);

  assign out = {arg0, clk};     // foo.mlir:2:8, :3:3
endmodule

module icmp(    // foo.mlir:6:1
  // input  /*Zero Width*/ a,
     output                y);

  assign y = a == a;    // foo.mlir:7:8, :9:3
endmodule

module parity(  // foo.mlir:12:1
  // input  /*Zero Width*/ arg0,
     input                 clk,
     output                out);

  assign out = ^arg0;   // foo.mlir:13:8, :15:3
endmodule

[mortbopet]

@uenoku the intention with this commit is to align on how we want this pruning infrastructure to work, when to apply it in the emission preparation pipeline, plus a small pruning example which demonstrates the method (... which also happens to unblock me :)). There's loads of things that both need to be pruned (for correct emission, as you provided examples of) as well opportunities for pruning which simplifies ExportVerilog. But i'd prefer to keep those in a follow-up commit once we've aligned on how to do it.

[teqdruid]

• Pruning doesn't erase anything but instead just marks an op with a pruning reason. This is then a signal to the emitter that an op should be commented out. This allows for good traceability (letting the emitter export verilog for the pruned logic) as well as provide a structcured way of signaling to the emitter that a given operation should be commented out. This method should be able to be leveraged to rewrite most, if not all, of the zero width emission logic currently in ExportVerilog.

I would prefer that ops be deleted. I could see this adding a bunch of complexity to ExportVerilog. For example, in your ExportVerilog change, what if some other emission logic emits a newline? If traceability is important to you, I think the principled approach is to replace the ops with verbatim comments.

[mortbopet]

I would prefer that ops be deleted. I could see this adding a bunch of complexity to ExportVerilog. For example, in your ExportVerilog change, what if some other emission logic emits a newline? If traceability is important to you, I think the principled approach is to replace the ops with verbatim comments.

Yea, so that's the tradeoff - verbatim comments requires ops to be printed in their MLIR form. Which would then requires the user to look at the accompanying MLIR IR file that is emitted by the tool alongside the exported verilog, to inspect the zero-width trace. This seems like a big no-no given that the emitted SV file will be hard to parse and understand for an RTL designer, who's expecting readable SystemVerilog, and that only that.

I don't think this approach will add significant complexity to ExportVerilog. You are right about the issue of newlines - a fix for that would seem to be to emit a block comment here opened and closed around statement emission - added in 5bba7b6. @uenoku thoughts on block comments?

[mortbopet]

Following some offline discussion with @teqdruid, we don't see an immediate need for emitting an SV-style comment trace in the generated output for the pruned zero-width logic. As such, reverted the implementation back to pruning-by-removal, without any replacement sv.verbatim comments (due to the nonsensicality of emitting MLIR IR in SV). Will push this for now, but open to opening an issue if there eventually is an argued need to have SV-style comment traces for the pruned logic.