Bug 765 - Add way for instructions to specify in csv if they have OE field
Summary: Add way for instructions to specify in csv if they have OE field
Status: CONFIRMED
Alias: None
Product: Libre-SOC's second ASIC
Classification: Unclassified
Component: source code (show other bugs)
Version: unspecified
Hardware: PC Linux
: --- enhancement
Assignee: Luke Kenneth Casson Leighton
URL:
Depends on:
Blocks:
 
Reported: 2022-01-18 04:21 GMT by Jacob Lifshay
Modified: 2022-01-18 18:39 GMT (History)
1 user (show)

See Also:
NLnet milestone: ---
total budget (EUR) for completion of task and all subtasks: 0
budget (EUR) for this task, excluding subtasks' budget: 0
parent task for budget allocation:
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jacob Lifshay 2022-01-18 04:21:57 GMT
currently there's a hard-coded list:
https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/decoder/power_decoder2.py;h=a636fdf5160cbfa680cdbf8ea36bced0304ca7f7;hb=HEAD#l573

that should be replaced with reading a field from the CSV, I had to manually add grev to the list after spending 2 hours debugging why overflow being enabled when it shouldn't.

Options should include:
never writes overflow (what I needed for grev, also ternlogi)
overflow calculated by pseudocode (like how division instructions do it)
overflow calculated from inputs/outputs based on OE field (like how add. addo. do it)
Comment 1 Luke Kenneth Casson Leighton 2022-01-18 07:00:31 GMT
https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=openpower/isatables/minor_5.csv;h=c0260dbf65e18ac3d4c1abe623becf5c77159810;hb=b55f37ee14b3253087f0a743eb795b53e59f25d2

   3 0010010110-,SHIFT_ROT,OP_GREV,RA,RB,NONE,RT,NONE,CR0,0,0,ZERO,0,NONE,0,0,0,0,0,0,RC,0,0,grev,X,,1,

notice how you've requested "RC" post-processing?
   
    0,-->RC<--,0,0,grev,X,,1,

RC and OE typically work together and are really only supposed to be
applied to arithmetic operations (not logical ones).  the reason why
microwatt specifies them together is because in those arithmetic
operations, you process RC=1 and OE=1 together including reading
and writing of XER.SO, where only under certain conditions is XER.SO
_not_ written to... but that cannot be determined at issue-time
(i explained this in another bugreport about 6 weeks ago)

i wondered a year ago when fighting with understanding on this whether
to add an OE field to the CSV files, you can see the link to the archive.
in the end decided against it because it would deviate from what microwatt
does.

in my mind the question is not entirely about whether the CSV files should
support a separate OE encoding but whether grev should have an RC=1 option
in the first place: if it was a bug due to cut/paste then this bugreport
should be closed as "INVALID".

Rc=1 is kiiinda also supposed to only be used for arithmetic operations,
[numerical integers - obviously grev in no way fits that category]
and because any that violate that rule are exceptional, a case could be
made to *not* have a CSV OE encoding, such that instead the source code
for PowerDecoder2 is in fact *forced* to have an explicit documented
explanation as to why the exceptional circumstances exist. trying to
fit that explanation into CSV file format is completely inappropriate,
obviously.

now, if on the other hand it turns out that a large number of the
bitmanip instructions being added *do* need - as a group - an Rc=1
field but no OE - then that's a different matter.  however that's
best discussed separately, under a completely different bugreport,
evaluating whether the bitmanip instructions should have Rc=1.
most of them won't, because they don't calculate standard
arithmetic-integer-numeric results. GF will be a bundle-o-fun to
work _that_ one out, what does GT and LT even mean, in GF arithmetic.
Comment 2 Jacob Lifshay 2022-01-18 07:04:44 GMT
a large number of bitmanip instructions have Rc but not OV -- nearly every binary bitwise logic op in the base OpenPower instruction set:
and, and., but no ando
all the other ones: or, xor, eqv, andc, orc, nand, nor
Comment 3 Jacob Lifshay 2022-01-18 07:06:50 GMT
imho microwatt treats Rc and OV together just because they can get away with it, not because there's some deep reason why that's done.
Comment 4 Jacob Lifshay 2022-01-18 18:39:59 GMT
additional reasoning why this shouldn't be closed as invalid:
https://bugs.libre-soc.org/show_bug.cgi?id=755#c26

(In reply to Jacob Lifshay from bug #755 comment #26)
> (In reply to Luke Kenneth Casson Leighton from bug #755 comment #25)
> > you've accidentally requested RC in the CSV file for this instruction,
> > which inherently and automatically requests OE as well.  that's a bug
> > in the CSV file, not the simulator.
> 
> no, it's a bug in the simulator that there isn't a way to request RC without
> OE unless you add GREV to a magic list of exceptions -- those exceptions
> should be translated to an OE field in the CSV (they only cover instructions
> without OE where the XO-form OE bit is 1; instructions like nand without OE
> but with the XO-form OE bit set to 0 aren't included in the list of
> exceptions because mis-interpreting the XO-form OE bit works even though
> it's not an XO-form instruction.