-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add EnableSignal, useful for making Instances compatible with EnableInserter #285
Comments
Unfortunately it is not possible to write an equivalent of For (a), consider that it would have to know to replace a DFF with a DFFE and hook up the |
Of course, if you are asking for an ability to customize the action of |
I do think the (b) option to have platform specific clock gating primitives is a good solution. One can then also error when the platform does not provide this feature and a domain |
I'm not convinced here at all. I think it's worth exploring, but I see many ways this can potentially go wrong, and in general,
I agree that emitting a diagnostic when |
Couldn't you change |
And maybe also a |
Ah I see, so your complaint isn't that the code that uses Yes, I think changing the semantics of |
What I want is that if a nMigen |
nMigen has no idea which ports of the instance are clocks, resets, or enables, nor does it have any idea that it should e.g. turn DFFs into DFFEs. I don't understand how this feature can work without any additional effort spent while defining an |
nMigen does know that output of |
Sure, I'm fine with adding |
To me the |
I see. Do I understand it correctly that you're suggesting that |
I would only introduce |
There are a few issues here. First, many FPGAs do not have a clock gating primitive. What should iCE40 do here, for example? There is no vendor-sanctioned way to gate a clock, and even worse, I don't think either the FOSS tools or the proprietary tools would be able to cope with the timing consequences of you abusing a LUT as one, even if you managed to find a truth table where it would not glitch. Second, FPGAs that do have clock gating primitives typically have severe restrictions on their use (e.g.: you only get one per chip and nMigen already uses it by default, or there are placement restrictions, or there are timing restrictions, or they aren't glitchless, etc). So effectively this auto-gating feature for Third, I don't think your suggestion:
is viable even on ASICs. Consider that not every input of an |
I do think yosys needs to be extended to handle the clock gating primitive and the primitive does not necessarily have to translate directly to a real physical cell on the FPGA. I'm not familiar with ice40 primitives but if the registers have an enable input the clock gating primitive could translate to an extra condition on the enable signals on all registers connected to the output of the clock gate. Worst case the enable has to be implemented as a mux on the input as you now do for the nMigen handling of
How is this different from |
Remember that nMigen does not only target Yosys. When used with non-FOSS toolchains, you cannot emit this primitive, since it'll just be an unknown Verilog cell that e.g. Lattice iCECube will fail on. In the future I would also like to target other IRs like FIRRTL (since Yosys RTLIL does not have a strict specification with a strong compatibility guarantee), which introduces the same issue.
I am completely opposed to adding a core primitive that will be a no-op on all currently supported FPGAs and therefore introduce a sim/synth mismatch. I am also strongly opposed to adding any core primitive that requires platform support at all (i.e. does not translate to portable synthesizable Verilog), and you have not shown why this case is so important that it requires doing so. I do recognize that it might unfortunately be necessary in some other circumstances, though.
The existing
nMigen does not currently have a lot of special handling for clocks themselves; other than when adding logic to a clock domain, |
I have wrapped some retro cores like motorola 68000, Z80, mos6502 implemented in VHDL & Verilog in nMigen. Plan is to distribute them through pypi and conda. For users they should behave as other nmigen block including |
But you don't need a core primitive that automatically introduces clock gating to make such instances work with Adding a (That's leaving aside my view that it is impossible to robustly implement your suggestion as described because there is no way to determine which inputs should be gated and which should not.)
I think nMigen should certainly have support for clock gating, similar to how it already has support for e.g. DDR I/O. I do not think this support should be a part of the core language; rather, it should be a part of the platform support. |
I think adding |
@Fatsie By the way, I especially like your suggestion of adding Also, the approach I described above (using an environment for late bound signals) will let you easily implement a |
I am making wrappers around existing RTL code using
Instance
. These block have a clock input. This structure seems to not be compatible withEnableInserter
. This issue is to see if there is a way to make it compatibl or alternatively at least have a warning or error when usingEnableInserter
.Reduced example code is this:
Giving the output:
The
ce
signal is not used.The text was updated successfully, but these errors were encountered: