[FIRRTL] Lower registers under ifdefs #8605
Conversation
|
|
||
| // Create our new property for field 0. | ||
| auto sym = StringAttr::get(context, innerSymNS.newName(hint)); | ||
| auto visibility = StringAttr::get(context, "private"); |
There was a problem hiding this comment.
FWIW I believe this might be first use of visibility re:inner symbols (first that isn't default of 'public').
Probably we should remove this field until it has meaning.
Looks like some tests make use of it, but I don't think we create any that aren't "public".
Anyway, since it doesn't mean anything presently they aren't /not/ private either so... 👍 .
| // Build the new list of inner sym properties. | ||
| SmallVector<hw::InnerSymPropertiesAttr> properties; | ||
| if (attr) | ||
| llvm::append_range(properties, attr.getProps()); |
There was a problem hiding this comment.
Since the new inner symbol is on lowest fieldID anyway, maybe can just append existing props after it and be done?
If there's any doubt, this is fine too 👍 .
| @@ -107,53 +115,273 @@ void FirRegLowering::addToIfBlock(OpBuilder &builder, Value cond, | |||
| } | |||
| } | |||
|
|
|||
| /// Attach an inner-sym to field-id 0 of the given register, or use an existing | |||
| /// inner-sym, if present. | |||
| static StringAttr getInnerSymFor(InnerSymbolNamespace &innerSymNS, | |||
There was a problem hiding this comment.
Sorry you had to roll your own thing for this! 🙃
| @@ -8,6 +8,7 @@ | |||
|
|
|||
| #include "FirRegLowering.h" | |||
| #include "circt/Dialect/Comb/CombOps.h" | |||
| #include "circt/Dialect/FIRRTL/FIRRTLUtils.h" | |||
There was a problem hiding this comment.
Is this an okay dependency? Do these conversions already depend on the FIRRTL dialect? If not, there may be places that need to be updated for that.
There was a problem hiding this comment.
I included this header for the parallel transform/reduce utility defined there, not for anything related to the FIRRTL dialect. I've moved the transform/reduce helper to a common circt utility header and included that instead.
rwy7
force-pushed
the
fix-reg-under-inline-layer
branch
from
9b08888 to
7af7812
Compare
rwy7
force-pushed
the
fix-reg-under-inline-layer
branch
2 times, most recently
from
56e2e54 to
fb0402f
Compare
rwy7
force-pushed
the
fix-reg-under-inline-layer
branch
from
fb0402f to
3039c4e
Compare
In seq-to-sv, we transform seq FirRegOps to sv RegOps. This is done using a helper called FirRegLowering. This PR changes FirRegLowering to correctly lower registers under ifdef blocks. The initialization of registers is placed in an initial block at the footer of the enclosing HWModuleOp. In order to initialize registers buried under ifdefs, we have to refer to these registers by a local XMR, because the buried registers will not dominate the initial block. We build these in a prepass over the MLIR module, returning a "PathTable" sending buried FirRegOps to their HierPathOps. The main FirRegLowering procedure operates in parallel on each HWModuleOp. Each invocation is now handed a const reference to the path table, which allows us to produce an XMRRefOp as needed while emitting the register initialization code at the footer of the HWModule. The initialization code for a register has to be guarded under the same ifdef conditions as the register itself. To do this, while lowering registers, we record the conditions under which the register is defined, and recreate them to guard the initialization code. When we lower a register, we attempt to transform any input mux tree into an SV if-else tree of connects. This PR modifies the if-else generation to co-locate the if-else with the register. So if a register is guarded under an ifdef, then the if-else structure driving it will also be under the same ifdef.
rwy7
force-pushed
the
fix-reg-under-inline-layer
branch
from
3039c4e to
a913d1a
Compare
In seq-to-sv, we transform seq FirRegOps to sv RegOps. This is done using a helper called FirRegLowering. This PR changes FirRegLowering to correctly lower registers under ifdef blocks.
The initialization of registers is placed in an initial block at the footer of the enclosing HWModuleOp. In order to initialize registers buried under ifdefs, we have to refer to these registers by a local XMR, because the buried registers will not dominate the initial block. We build these in a prepass over the MLIR module, returning a "PathTable" sending buried FirRegOps to their HierPathOps.
The main FirRegLowering procedure operates in parallel on each HWModuleOp. Each invocation is now handed a const reference to the path table, which allows us to produce an XMRRefOp as needed while emitting the register initialization code at the footer of the HWModule.
The initialization code for a register has to be guarded under the same ifdef conditions as the register itself. To do this, while lowering registers, we record the conditions under which the register is defined, and recreate them to guard the initialization code.
When we lower a register, we attempt to transform any input mux tree into an SV if-else tree of connects. This PR modifies the if-else generation to co-locate the if-else with the register. So if a register is guarded under an ifdef, then the if-else structure driving it will also be under the same ifdef.
Fixes: #8461