[Libre-soc-dev] my current plan regarding simdsignal

[Jacob Lifshay]

On Sat, Oct 23, 2021, 07:51 lkcl <luke.leighton at gmail.com> wrote:

> (ok, replied privately to jacob, addressing how he is clearly upset) it
> shows in some of the language you use, jacob. hope it helps provide some
> context and clears things up.
>
> technical issues:
>
> globals are always problematic.  they have specific uses
> (mutex/semaphores, singletons, class factories for dynamic allocation and
> adaptation), beyond that they are a "red flag" especially in python.
>

In this particular case, I think it's fine, since the way it is used is
such that it's much more similar to how Python's super() function works:
If code is inside the `with SimdScope(...)`, then it will use that
SimdScope. If it's outside, then it will raise an error.

I originally got this idea from Rust's async design, where async functions
can get access to the state necessary to schedule the async function to be
woken up again after I/O by accessing a only-set-within-a-scope
thread-local variable.

I can change it to also use a thread-local (needed if code is multithreaded
rather than just multiprocess like usual for Python (multiprocess is
standard for tests)).

Because the variable is only accessed in a scoped fashion, it won't have
any problem with Python testing, since it will only be within that scope
when inside the test and won't conflict with other tests since they have
their own separate scopes. It automatically handles resetting the global
properly if the code inside the `with` raises an exception.

The idea is that each Elaboratable class that we refactor to use SimdSignal
will basically always wrap its elaborate() (and maybe __init__ too)
function's body in a `with SimdScope`, such that it's always pretty obvious
where the scope comes from.

>
> in this case, the use of a default (non-provided, null) parameter to "pick
> up" a global is particularly problematic.
>
> imagine a scenario where a newcomer creates code with 400 uses of
> SimdSignal, only 399 of which explicitly give a SimdScope and, buried in
> that, 1 that does not.
>

Then, that one that does not will error in the unit test since they aren't
using a `with SimdScope` since they decided to always pass SimdScope in
explicitly instead.

If desired, I could also add a `with SimdScope.none()` that allows forcing
SimdScope.get  to error even if your code is in a nested scope.

>
> it could be days before they find the error.
>

just as long as it takes for execution to reach that part of their code in
its unit test...

>
> i literally just used the technique of not setting a default very
> deliberately, to find a bug i was not expecting, when adding the lhs mode
> function, only 2 days ago.
>
>
> additionally, globals stop you from doing 2 or more things
> simultaneously.  we may wish to set up 2 HDL pipelines and run them through
> the test API side by side: a global prevents and prohibits that
> unnecessarily.
>

well, there are two cases, both of which it still works fine:

1. the two HDL pipelines are not nested -- they are side-by-side: since
each pipeline's elaborate will set up their own separate scopes, and
neither scope is inside the other, they won't conflict.

2. the two HDL pipelines are nested: the outer pipeline's elaborate will
set up its scope, then add the inner pipeline as a submodule, assuming the
inner pipeline's elaborate gets called then, then the inner pipeline will
also set up its own scope, SimdScope.get always returns the innermost
scope, so they won't conflict there, either. if the inner pipeline's
elaborate is delayed till after the outer pipeline's elaborate finishes,
then the scopes are no longer nested, just like case 1.

The only case we might have problems is if we use yield to suspend a
function while it's inside a `with SimdScope`, unless the generator is both
created and used entirely within the `with SimdScope` (which is true for
basically all cases). This won't cause problems with simulation --
SimdScope is intended for *creating* HDL, not for using in Simulator
processes.

Jacob