[Libre-soc-bugs] [Bug 838] sync or at least statically check fields.text, power_decoder, trans/svp64, CSVs between each other

[bugzilla-daemon at libre-soc.org]

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

--- Comment #29 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to Dmitry Selyutin from comment #28)

> I've never worked with SQL so I can neither confirm or deny it.

intuitively it works as it sounds, all fields are wildcard included.

> > btw there should be no defaults in PPCRecord.  the csv files are
> > required 100% to contain all entries
> 
> This is incorrect, both from CSVs point of view and sources.
> 
> CSVs (only a pair of entries, there're others):
> -----11111,FPU,OP_FP_MADD,FRA,FRB,FRC,FRT,NONE,CR1,0,0,ZERO,0,NONE,0,0,0,0,1,
> 0,RC,0,0,fnmadds,A,,,
> 0b1001000000,,,,,,,,,,,,,,,,,,,,,,,mcrxrx,X,

hmmmmm.... yep, hadn't spotted / forgot that.  been a while.

> > and then of course in finding an opcode match do:
> > 
> >     if (opcode & mask) == value: # match
> 
> Clarified in IRC; however, since we're going to need it anyway, I'll either
> have some class with opcode and mask properties, or simply will extend the
> Opcode with mask property.

if the list is compacted by a function down into a single
tuple of 32 bit (mask, value) by reconstruction the PowerDecoder
may be drastically simplified to a single for-loop creating one
single giant nmigen Switch/Case onto a Memory (ROM) lookup.

0,6, MAJOR would map to 0b0000...0111111 0b0000...NNNNNN
and be ORed with other entries in the list, similarly
mapped.



> (In reply to Luke Kenneth Casson Leighton from comment #27)
> >  364             fields += [(self.__section.opcode, 0, 5)]
> > 
> > that can be converted to (mask, value) form
> 
> I took it from binutils (instruction() function). I'd prefer this form as it
> fits fields.text better, and is really obvious (unlike masks).
> 
> >  368         if self.name.endswith("."):
> >  369             fields += [(1, 31, 31)]
> > 
> > ah no.  definitely not.
> > 
> > 1) some Rc=1 instructions *do not* use bit 31
> > 2) some instructions (stcix. or atomics somewhere) it is implicit
> >    (i.e. there is no non-Rc version)
> > 3) Rc=1 field extraction is often done elsewhere i.e. by manual
> >    recognition of the bit.
> 
> Thanks for this clarification! It seems we cannot avoid dealing with
> fields.text. That said... this is also part of information about the
> instruction, so it's reasonable.

power_fields.text, yes.  you have to perform a lookup of the instruction
mnemonic first, followed by finding the Form (X-Form, D-Form) *then*
you can look up Fields reliably.

(this is why decoding is done in two stages, decode1 and decode2)

bear in mind that this all comes from microwatt decode1.vhdl and
decode2.vhdl.

those HDL files were created so as to not have Absolute Hell On Earth
which if you have seen gem5's hand-created Power ISA decoder tens of
thousands of lines long with nested switch statements you start to
appreciate.

however....

decode1.vhdl was NOT designed to be absolute 100% fully automated
i.e. equivalent to binutils objdump.

thus *neither are the csv files*.

an example is mtmsr[d] there is *only one OP*, OP_MTMSR
and inside the PIPELINE the required bit, L, is MANUALLY
decoded to discern between these two instructions.

the RC column in the CSV files is expected to be used in
this same way and *also includes OE detection* as a note
*to the PIPELINE* (and also decode2.vhdl aka PowerDecoder2)
to perform further *manual* decoding of both Rc and OE.

i relate this because efforts to turn the csv files into
a full automated objdump have consequences and a high cost,
not just in coding terms but also deviation from microwatt.

i have no problem with auto-generating extra field entries
that go into class instances but drastic modification of
the CSV files is to be strongly discouraged.  as is using
JSON or other "modern" format for the same reasons.

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