We intended to keep the InstanceGraph up to date, but when we
completely erase FIRRTL ClassOps, we were not removing them from the
InstanceGraph. This can lead to invalid memory accesses, which I
noticed with ASAN while using the InstanceGraph in a pass downstream
of this.
The instance graph is transitioning towards supporting multiple possible targets for instance operations.
As a consequence, it can no longer assume that there are instance operations with a single target and it will expose a generic interface returning a list of targets.
This PR removes the lookup helper from the graph and instead provides the users (`firrtl::InstanceOp` in particular) with helpers to fetch the unique referenced instance.
This changes FreezePaths to only follow paths up to the root of the original
hierarchical path operation, instead of continuing upwards in the instance
graph. We no longer enforce that the path is unambiguous up to the top level
(or public) module. This is in preparation to support paths when there is
multiple module instantiation, where we will enfore that paths are rooted at
their owning module.
In an effort to support multiple instantiation of modules, we need to overhaul
the ABI for paths. Before, it was accidentally assumed that only public or
top-level modules could be multiply instantiated, and base-paths would be
rebased at those boundaries. Now, when lowering modules to classes (but not
when classes are lowered to classes), the basepath is always rebased. This
lets us compose paths across multiple module instances.
The Map property type was not necessary in the end. A Map can easily
be represented by a list of object with two fields, and this is how
its been done downstream. Remove this to simplify FIRRTL and reduce
maintainence burden.
This PR does an overhaul of how path related operations work in the OM dialect.
When OM classes are created from FIRRTL, a base path is now fed through the
class hieararchy. All paths to hardware objects are now created relative to
the base path. The `path` op has been renamed to `path_create`, and now takes
a basepath SSA parameter, which means that it is impossible to create a path
which is not relative to some base.
A second set of operations to create "frozen paths" have been added. In
FreezePaths, we lower all path operations to hard-coded strings, which allows
us to remove the hardware from the OM IR and still be valid. These operations
have been changed to return `FrozenPathType`, which is to make sure that people
don't try to inter-mix the two kinds of path types. The intention is that the
evaluator will only understand frozen paths.
The PathAttr in OM no longer represents the entire target path, but just the
list of instances which lead up to it.
This also adds an OM utility for parsing FIRRTL style target strings, which is
used in the `frozenpath_create` MLIR assembly format. The existing APIs
contained in FIRRTL were not suitable since these new strings do not include
the circuit name, and we need some restrictions for parsing base paths, which
can not target components. The new parser is a bit more resilient to badly
formed path strings. In the future it may be possible to reunite these two
bodies of code.
The existing python support for paths had to be removed.
While lowering an ExtModule, if it has a defname set, use that for the
lowered ExtClass name. This is needed to ensure the lowered ExtClass
lines up with the lowered Class.
This also plumbs around the SymbolTable, so we can use it to
efficiently look up the referenced module while lowering instances,
and check that for a defname when we build the name of the class for
the object instance.
Double-precision floating point type.
Add attribute, constant op, and parse + emit support.
Only support what we understand as floatingpoint in lexer for now, which includes exponent notation.
Add support to LowerClasses.
The FIRRTL AnyRef type is converted to an OM Any type, and FIRRTL
AnyRef casts are converted to OM Any casts. This is a straightforward
conversion of the same concepts into the OM dialect.
This adds lowering for extmodule similar to extclasses and module: an
external class declaration is created with the property input and
output ports. This also adds support for converting instances of
modules or extmodules that have been lowered, and keeping the instance
graph up to date.
A small addition is also included to always create a class for public
modules and extmodules, which is a further refinement of the as-yet
undocumented ABI.
Property wires are added in some scenarios for convenience. This would
be a good canonicalization pattern to have, but canonicalization
shouldn't be needed for correctness, and this isn't always possible in
non-graph regions. So, this pass explicitly handles wires of
properties when they are known to be in graph regions, by simply
finding the single assignment to the wire, using that value instead of
the wire, and removing the assignment.
Closes#6051.
- Parser support in the FIRParser
- New ExtClassOp in the firrtl dialect for extclasses
- New ClassLike op interface in firrtl
- LowerClasses:
- Lower firrtl.extclasses to om.class.extern
- Legalize om.class.extern.field ops
OM path operations use a HierPathOp to identify their target. To facilitate
this, it makes sense to insert OM classes under the FIRRTL circuit, as
HierPathOp should be under the same symbol table as the path operation for
lookup to work correctly. If they were not under the same symbol table, it
would be possible to craft a nested symbol reference, but that would just
complicate the LowerToHw pass.
This change removes the restriction that OM ClassOps have MLIRModuleOp as a
parent, changes LowerClasses from a MLIRModule pass to a FIRRTL CircuitOp pass,
and updates some logic to ensure that we are inserting the class operations in
to the correct location.
As a quick example of how this will look with path operations:
```mlir
module { // SymbolTable
firrtl.circuit "Foo" { // SymbolTable
hw.hierpath @Path [@Foo::@Wire]
firrtl.module @Foo { // Not a SymbolTable
%wire = firrtl.wire sym @wire : !firrtl.uint<1>
}
om.class @Class() { // Not a SymbolTable
%0 = firrtl.path @Path
}
}
}
```
This adds the LowerClasses pass right before LowerToHW. With this change, we
can get a lowering of FIRRTL with properties right through the pipeline with
Verilog output.
Tweak op legality predicate to reject if recursively contains any firrtl types.
Without this the conversion accepts incomplete conversion
(`!om.list<!firrtl.string>`) when it should reject it.
This is a fairly significant rewrite, so I've chosen to rename the
pass, but otherwise share some of the good parts. A lot of warts are
hopefully removed.
The pass lowers both FIRRTL modules with property ports as well as
FIRRTL classes into OM classes. It does a linear walk to initially
create the OM classes, and then processes each in parallel.
The logic to move ops into the new class body is greatly
simplified--ops are copied over, in order, if they have property
operands or results. We might make this even simpler with a trait at
some point.
Instantiations and the instance graph no longer need to be kept in
sync--only FIRRTL Object instantiations are effected. These are
converted to OM Object instantiations. It's actually impossible to
represent a FIRRTL Object that takes inputs with the current
flow-checking. I tried to write out an implementation for it, even
though I can't test it.
Besides the structural changes, I've also added type conversion from
the FIRRTL ops to the OM versions. I'm using DialectConversion for
this, which actually helps for this sort of thing. There are fairly
simple patterns for converting the constant property ops to the OM
versions. These could almost be templated on the operation class, but
minor differences made that more trouble than its worth. There are
also patterns for legalizing OM class fields and OM classes, which are
just there to let the type converter do its thing.
I've added a basic unit test for the kind of IR we want to support
here. One more parser and flow-checking support is in, I will also
include an end-to-end test from FIRRTL through ExportVerilog to the
final MLIR.
Megan Wachs