[Libre-soc-bugs] [Bug 745] OP_TERNARY instruction

[bugzilla-daemon at libre-soc.org]

https://bugs.libre-soc.org/show_bug.cgi?id=745

--- Comment #26 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Jacob Lifshay from comment #25)

> yup, it won't work that way, if pspec is a Mock instance here. Mock will
> return new Mock instances for any attribute access. Mock is plain and simple
> the wrong tool for the job, dataclasses are better.

Mock was there, it does the job, it saved 1 day writing a custom class
that does exactly the same thing.

now it will be 3-4 days going through the entire codebase removing it
and rerunning all unit tests.  which we csnnot afford to do.

this is about the lowest possible priority task of all the lowest
priority tasks and even discussing how great a replacement dataclass
should be by comparison is a waste of time.

if you had helped out at the time then you could have written such
a class.  but you did not help.  therefore under extreme pressure
i had to make drastic shortcut decisions and now we are stuck with it.

lesson learned, i trust.

> From my reading through the code, it appears as though the pspec passed into
> ShiftRot isn't the same pspec created for the whole core, so there shouldn't
> be an issue for that particular use case.

yes, annoyingly confusing choice of variable name, 18 months ago.
i think i meant to merge them, then changed my mind, and never corrected
the variable names.  now there are hundreds of pspecs in use and it will
be a pig to separate them all.

> (hasattr(pspec, "attr") and pspec.attr) isn't any better than getattr(pspec,
> "attr", default), cuz hasattr always returns true for any attr string.

now you know why i did "pspec.somevar == True" not "if pspec.somevar"
because that will always return true.

it does need to be *in* TestMemPSpec and passed through, not in the
pipeline spec (sorry about the confusion) because TestMemPSpec
is what gets set/activated from the commandline (and various unit
tests).

in this way there is a choice to compile with or without draft
bitmanip.

exactly the same as nosvp64 and other options.
see issuer_verilog.py for the full list, an option
is needed --with-draft-bitmanip

by default it should be enabled (incl in test_issuer.py)
because it will be a pain to type every time.

-- 
You are receiving this mail because:
You are on the CC list for the bug.