circt icon indicating copy to clipboard operation
circt copied to clipboard

[HW] Enable `i0` for HWIntegerType

Open mortbopet opened this issue 3 years ago • 1 comments

Luckily a fairly minor change due to ExportVerilog already have good support for emitting i0-typed expressions as comments. This commit ensures that hw.array_get for singleton arrays may be emitted legally, which is probably the most pressing matter in embracing i0-typed values. Not a solution to every problem in the code-base related to this, but I think this is a reasonable starting point.

Changes:

  • zero-width mlir::IntegerType valid as HWIntegerType's
  • hw.constant generates i0 constants if provided an i0 type (no change)
  • hw.array_get now expects i0 type for singleton arrays (special-case in IndexBitWidthConstraint has been removed).
  • hw.array_get will always emit 1'b0 for i0-typed indexes during verilog export. The expression corresponding to the i0 index is emitted inline as a comment.
  • Some changes to FIRRTL subindex and subaccess lowerings

mortbopet avatar Sep 23 '22 09:09 mortbopet

Maybe we can change IndexBitWidthConstraint to accept both i0 and i1 to gradually support i0 values (I'm not sure it's a good idea though).

That would indeed have to be the tradeoff seeing as the most significant part of this PR is the side-effects of removing the special condition for IndexBitWidthConstraint.

mortbopet avatar Sep 23 '22 13:09 mortbopet

allow i1 or i0 for array indices.

Tinkering with this, it seems that there is no working around this requiring us to explicitly mention the index type of array_get in assembly since a non-ambiguous type can no longer be inferred from the TypesMatchWith trait.

@uenoku reverted FIRRTL change.

i0 and i1 types are now both valid for singleton arrays.

mortbopet avatar Sep 26 '22 13:09 mortbopet