dialects: (hw) Add hw.module and hw.output operations

[Moxinilian]

This PR introduces the hw.module and its companion hw.output from CIRCT, while adding various small enhancements to the general infrastructure to accommodate them. ModuleOps are used to specify self-contained hardware modules, akin to functions in software.

A hw.module looks like the following:

hw.module @name<param1: i1, param2: i2>(in %a: i8, in %b: i9, out c: i8) {
  %res = arith.constant 0 : i8
  hw.output %res : i8
}

While hardware modules have similar semantics to functions, both inputs and outputs in hw.module are defined within the parenthesis, as ports. This is because some ports may be both input and output in CIRCT (currently not supported). Modules can optionally receive static parameters (in the example, param1 and param2). Those are akin to C++ value templates or Rust const generics, and are useful to factor similar modules together.

DirectionAttr, ModulePort and ModuleType are defined to represent the type of a module and its ports. ParamDeclAttr is an attribute to represent the parameter definitions on the module.

The hw.output operation is the terminator for hw.module, setting the value of the output ports.

cc @superlopuh @math-fehr

[codecov]

Codecov Report

Attention: Patch coverage is 97.37991% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 89.68%. Comparing base (4516ae1) to head (4fd635a).
Report is 8 commits behind head on main.

Files	Patch %	Lines
xdsl/dialects/hw.py	97.37%	2 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2366      +/-   ##
==========================================
+ Coverage   89.63%   89.68%   +0.04%     
==========================================
  Files         346      346              
  Lines       41013    41418     +405     
  Branches     6093     6175      +82     
==========================================
+ Hits        36761    37144     +383     
- Misses       3360     3372      +12     
- Partials      892      902      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[luisacicolini]

looks good to me!

[lucjaulmes]

Thanks, nice contribution! A bit hard to get my head around all the various parts though. Here’s a few thoughts

[lucjaulmes]

AFAIU the corresponding class in circt is called HWModuleOp. Not sure how close we feel like sticking to upstream code.

More to the point, I think ModuleOp is lacking a few traits here. I’m thinking at least InnerSymbolTableTrait() and SymbolOpInterface() (from upstream HWModuleOpBase) and IsolatedFromAbove(), SingleBlockImplicitTerminator(OutputOp) (from upstream HWModuleOp).

[Moxinilian]

I am having issues with "InnerSymbolTableTrait", as it requires a "InnerRefNamespaceTrait" parent but I cannot find any reference to that trait on the entire internet. What is it?

[lucjaulmes]

The trait is just called InnerRefNamespace in the circt codebase – the suffix is added due to a name clash (there is also an InnerRefNamsespace class). The trait definition is in include/circt/Dialect/HW/HWOpInterfaces.h and include/circt/Dialect/HW/HWOpInterfaces.td. There’s some brief explanation about it in the “Traits and Classes” section of circt’s docs/RationaleSymbols.md document.

[lucjaulmes]

The operation that has that trait is the firrtl CircuitOp operation.

[Moxinilian]

Oh of course! Right. I'm just going to add it to the module operation, hopefully this makes sense.

[Moxinilian]

The operation that has that trait is the firrtl CircuitOp operation.

That does not sound right, HwModules can live in builtin.module too. I think they may also be adding it to MLIR's ModuleOp via an external model, but I cannot find where this is.

[Moxinilian]

Ah no, I think the problem is in the InnerSymbolTable trait implementation in xDSL. It does not check for an InnerRefNamespaceLike but an InnerRefNamespace. I can probably fix that.

[Moxinilian]

Actually I don't understand what is happening in InnerSymbolTableTrait, so if you don't mind I just will not add the trait for now. My understanding is that the implementation of InnerSymbolTableTrait is not complete enough to be used anyway.

[lucjaulmes]

I think that’s fine, let’s not make this PR to large. I suppose the propoer way to add it later is to just add a comment and file a follow-up issue. LGTM for the rest.