You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Python standard practice is to use isinstance(v, C) instead of type(v) is C. This allows using user-defined classes that derive from nmigen types.
One place that needs to be fixed is
First, there is no support for deriving classes from nmigen types other than Record. This is already evident by the fact that it uses type-driven dispatch through the use of a visitor pattern. (If I have to drive the point in by overriding __subclass__ then I will do so.) Therefore, deriving classes from nmigen types is not an argument for making this change.
Second, although some of those type() calls can be turned into isinstance() calls, there are many which will not be, and that is the ones in Simulator._run_process. I've benchmarked the simulator and it became measurably faster by avoiding isinstance().
If you use git blame, you will see that the commit introducing all of the type(c) dispatches was 7341d0d, and the purpose of them is to make back.pysim faster. If you profile the nMigen testsuite, you will see that it spends a lot of time in AST transformations--in fact if the actual testbench runs only for a few cycles of simulated time, the time spent in AST transformations exceeds that of the time spent in simulation.
If you look at the profile before that commit, you will see that isinstance is a hot spot. To address this, I've turned most of these isinstance calls (except for Record--but there are far more Values that are not Records) into type calls, which made it a bit faster. Since subclassing nmigen types (other than Record) is unsupported, this is a perfectly safe optimization and I see no reason to change anything here.
In some cases, nMigen uses type() instead of isinstance() to dispatch
on types. Make sure all such uses of type() are robust; in addition,
make it clear that nMigen AST classes are not meant to be subclassed.
(Record is an exception.)
Fixes#65.
Python standard practice is to use
isinstance(v, C)
instead oftype(v) is C
. This allows using user-defined classes that derive from nmigen types.One place that needs to be fixed is
nmigen/nmigen/hdl/xfrm.py
Line 83 in 6a77122
See
http://bugs.libre-riscv.org/show_bug.cgi?id=64#c10
http://bugs.libre-riscv.org/show_bug.cgi?id=66
https://stackoverflow.com/a/1549854
The text was updated successfully, but these errors were encountered: