CXXRTL should warn or fail on unsupported clock features

[povik]

Version

Yosys 0.49+3 (git sha1 916fe99, clang++ 11.1.0 -fPIC -O3)

On which OS did this happen?

macOS

Reproduction Steps

Use CXXRTL on a design with a non-trivial clock tree: e.g. with a clock mux or a clock gate on the clock path; the design is likely to be missimulated due to #3549.

Expected Behavior

We should emit a warning or refuse the process such design.

Actual Behavior

CXXRTL processes the design, the design can be missimulated

[povik]

@whitequark What's the right way to condition this? #3549 describes the issue as clocks which are not module inputs. What about clocks which are module inputs on a nested module, but are connected e.g. to a clock divider in an outer module and CXXRTL is run in hierarchy preserving mode?

Is it OK by you if we make CXXRTL flat out refuse to process the impacted designs?

[whitequark]

Non-flat netlists are completely broken when clocks are involved for the same basic reason, except there is no workaround for that case. We should emit the same warning for them (but not completely disallow, since blackboxes rely on non-flat netlists...)

Is it OK by you if we make CXXRTL flat out refuse to process the impacted designs?

I feel like that may be going too far. Imagine you have a single flop somewhere that you really don't care about but can't easily remove. Now your entire design can't be simulated.

I feel that a loud warning is, at least, a major improvement, and fixing this issue entirely is the ideal way forward. If I did not think it could be fixed then of course rejecting would be the right move.

[povik]

Non-flat netlists are completely broken when clocks are involved for the same basic reason, except there is no workaround for that case.

So the logic for the warning would be: if we find a clocked element (flop or clocked memory port) connected to anything other than a top-level module input (incl. non-top-level module input), we emit a warning referencing issue #3549. Sound good?

[whitequark]

Correct! This also applies for inputs marked cxxrtl_edge (if I recall the attribute name right). People have found workarounds to use blackboxes anyway so it should be a warning to avoid breaking existing flows.