circt
circt copied to clipboard
[FIRRTL] Reject hierarchical paths on `EICG_wrapper` in `LowerIntrinsics`
@mwachs5 has uncovered an issue in LowerIntrinsics when she ran the following command:
firtool test/Dialect/FIRRTL/extract-instances.mlir --ir-fir --fixup-eicg-wrapper
This errors out because a EICG_wrapper instance is targeted by a non-local annotation (and thus an hw.hierpath). We probably want to completely forbid annotations on clock gates, since we can't really annotate the intrinsics on the FIRRTL side. In practice it seems like we're not adding any annotations to clock gates. It would be good to (1) accept EICG_wrappers with and without test_en port in LowerIntrinsics, and (2) to error out if the intrinsic instance is targeted by a non-local annotation.
If we end up hitting that error and we see NLAs on clock gates in practice, a viable alternative might be to create a firrtl.wire after the clock gate to carry that annotation.
Yea, new things shouldn't support annotations (and intrinsics shouldn't/don't). I assume the medium term solutions is moving Chisel to directly generate the intrinsic instead of having a magic extmodule?
Yes exactly. The extmodule-to-intrinsic path exists to switch firtool over to the intrinsics without requiring upstream Chisel code to do the same in lockstep. Once everything works in production, we can switch Chisel code over to the intrinsics.