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

[bugzilla-daemon at libre-soc.org]

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

Luke Kenneth Casson Leighton <lkcl at lkcl.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://bugs.libre-soc.org/
                   |                            |show_bug.cgi?id=339,
                   |                            |https://bugs.libre-soc.org/
                   |                            |show_bug.cgi?id=340

--- Comment #24 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Jacob Lifshay from comment #23)
> (In reply to Luke Kenneth Casson Leighton from comment #22)
> > (In reply to Jacob Lifshay from comment #21)
> > > I also moved ternlogi to the shiftrot pipe 
> > 
> > great.  really, a compile-time option needs adding which can disable
> > it.  added to pspec.
> 
> I added that to shift_rot's pspec, I have yet to figure out how to pass that
> in from all the places where ShiftRot pipelines are created...will try to
> figure that out.

for anything related to TestIssuer unit test running, as well as ISACaller
running, the "base" class is here:

https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/test/runner.py;h=b8378a93ee550c67ac4eb48097005e9f631f0879;hb=74adc2b94a4409162119157f3f75e05a06e4c841#l135

which for TestIssuer simulations the extra argument must be passed here:

https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/simple/test/test_runner.py;h=962d47a20e9e11eaf992c91c5e424c4c7bbdae5f;hb=20ffa8ec223be44fc37df5184ca09d648e85a844#l292

and a command-line argument added here:

https://git.libre-soc.org/?p=soc.git;a=blob;f=src/soc/simple/test/test_issuer.py;h=b8245cbf1a5356f39a6695531ecda1e96460617b;hb=20ffa8ec223be44fc37df5184ca09d648e85a844#l37

(actually to all of these)

https://git.libre-soc.org/?p=soc.git;a=tree;f=src/soc/simple/test;hb=HEAD

hmmm TestMemPSpec must move into openpower-isa, hmmm.  importing from
soc in the openpower-isa repo is... Bad(tm)

then there will also be fu/compunit tests, as well as individual
pipeline-tests that need it enabling.

basically, anywhere you can find a "grep -r TestmemPspec"

> > (edit: or move the Cat() and add a comment about it setting "mode")
> 
> moved the Cat

great.  need to keep things together, comments are critically important
in a project of this size.

https://git.libre-soc.org/?p=soc.git;a=commitdiff;h=20ffa8ec223be44fc37df5184ca09d648e85a844

+def get_pspec_draft_bitmanip(pspec):
+    return getattr(pspec, "draft_bitmanip", False)

it can't be done that way (at least, i don't think so, can you please
check and put in a code-comment if it does)

TestMemPspec actually is a subclass of unittest Mock() which has overrides
on __getattr__ etc. and consequently you cannot just access attributes
in the "normal" way.

you'll need to check that getattr() and hasattr() do the same thing:
they probably don't, which is most likely why i specifically used the
(odd-looking) "if hasattr(pspec, "attribute") and pspec.attribute == True)"

 class ShiftRotMainStage(PipeModBase):
     def __init__(self, pspec):
         super().__init__(pspec, "main")
+        self.draft_bitmanip = get_pspec_draft_bitmanip(pspec)
         self.fields = DecodeFields(SignalBitRange, [self.i.ctx.op.insn])
         self.fields.create_specs()

please comment that.  explain what is going on (that it's draft instructions,
include the URL of draft bitmanip instructions as well) and also
cross-reference
at the top code-block back to the URL of the relevant bugreport comment as
to what work is going on here: something like "draft bitmanip instructions
are added here because of similar register profiles, they are not approved yet
by OPF, and can be disabled at compile-time.  draft spec is at {URL},
bugreport is at {URL#comment}"

this is so that we don't get completely and hopelessly lost even during
code-review right now, as well as later when other people wonder what the
heck is going on.

hmmm also can you please add this URL, it's missing
https://bugs.libre-soc.org/show_bug.cgi?id=339
and that it implements Power ISA v3.0B Book I Section 3.3.14 p101-110

(shiftrot was one of the very early pipelines and the practice of linking
the context (bugreport, Spec page numbers) hadn't been established yet).

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