Bug 887 - implement fmvis and 2nd-half variant
Summary: implement fmvis and 2nd-half variant
Status: RESOLVED FIXED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Source Code (show other bugs)
Version: unspecified
Hardware: PC Linux
: --- enhancement
Assignee: Konstantinos Margaritis (markos)
URL: https://libre-soc.org/openpower/sv/in...
Depends on:
Blocks: 234
  Show dependency treegraph
 
Reported: 2022-07-12 10:14 BST by Luke Kenneth Casson Leighton
Modified: 2022-10-07 11:45 BST (History)
2 users (show)

See Also:
NLnet milestone: NLNet.2019.10.031.Video
total budget (EUR) for completion of task and all subtasks: 2500
budget (EUR) for this task, excluding subtasks' budget: 2500
parent task for budget allocation: 234
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:
lkcl={amount=500, submitted=2022-09-12, paid=2022-09-15} markos={amount=2000, submitted=2022-07-27, paid=2022-08-11}


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Kenneth Casson Leighton 2022-07-12 10:14:23 BST
implementation hints:

* https://libre-soc.org/openpower/sv/bitmanip/ for the XO field
  to go into minor_22.csv
  https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=openpower/isatables/minor_22.csv;hb=HEAD
* see commit-diffs in https://bugs.libre-soc.org/show_bug.cgi?id=863
  for tips on files to be changed
* make sure sv/trans/svp64.py supports both "fmvis" and "sv.fmvis",
  see the end-of-file very rudimentary lst
* copy the DX-Form into sv/trans/svp64.py when adding the ".long" translation
  https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=openpower/isatables/fields.text;hb=HEAD#l51
* https://libre-soc.org/openpower/sv/int_fp_mv/#fmvis
  for spec including source code
* run "pywriter noall av" after each edit to av.mdwn, to produce
  av.py
* run "sv_analysis" after adding to major_22.csv and do a *separate*
  commit specifically for it.

edit these when done each stage:

* DONE: major_22.csv entry
* DONE: add entries into power_enums.py (string)
* DONE: add "exception recognition" in ISACaller (isacaller.py)
* DONE: sv/trans/svp64.py entry (recognising opcode, turning into ".long")
* DONE: add pseudocode (to av.mdwn) see spec page
* DONE: add scalar unit test test_caller_fmvis.py
* DONE (in same fmvis.py): add svp64 unit test test_caller_svp64_fmvis.py
* DONE: update spec page to fishmv

before each commit is pushed FOR GOODNESS SAKE run a modicum of unit tests
to ensure no "damage" is done:

* power_fields.py check it produces no errors
* power_decoder.py check it produces no errors
* test_caller_alu.py >& /tmp/f grep for "Err" in output
* test_caller_setvl.py >& /tmp/f grep for "Err" in output
* "pywriter noall av" should in *no way* have any syntax errors
Comment 1 Luke Kenneth Casson Leighton 2022-07-12 10:24:58 BST
discussion:

https://libre-soc.org/irclog/%23libre-soc.2022-07-11.log.html#t2022-07-11T19:54:09

to get sv/trans/svp64.py to recognise that it means scheduling a
secondary task for Dmitry to also ensure that binutils has corresponding
support. that's not something that's coverable by this budget, so can
it please be left for another time, i don't want to overload Dmitry.

adding support for hexadecimal into svp64.py, though, no problem at all.
it should be in theory easy to add here:
    https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/sv/trans/svp64.py;hb=HEAD#l48
Comment 2 Luke Kenneth Casson Leighton 2022-07-12 10:34:27 BST
(In reply to Luke Kenneth Casson Leighton from comment #1)

> adding support for hexadecimal into svp64.py, though, no problem at all.
> it should be in theory easy to add here:
>    
> https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/sv/
> trans/svp64.py;hb=HEAD#l48

nope, here:

https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=8b17162fd38e163f522729efe3c863b04d0884a0

so that exact same place would be where conversion from float to
number would be performed, except it's more complex in the case
of fmvis because of the truncation (taking the top 16 bits of
a FP number).  that can be "hack-fixed" by adding in the instruction
asmcode as a 2nd argument to to_number() :)
Comment 3 Luke Kenneth Casson Leighton 2022-07-12 16:49:40 BST
added the "right" DX-Form for fmvis

https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=268363083f80ab7ec6afc2c8db32ea95bbde27ae
Comment 4 Jacob Lifshay 2022-07-12 18:02:20 BST
the existing vsx float load immediate instructions seem to only use integers, no fp constants in assembly:
https://gcc.godbolt.org/z/fhnnjhr61
generates:
xxspltidp 34, 1066192077
1066192077 is float32 1.1 bitcast to int32
Comment 5 Konstantinos Margaritis (markos) 2022-07-26 11:54:03 BST
This has been fixed in commit 51ebd21.
Comment 6 Luke Kenneth Casson Leighton 2022-07-26 12:53:10 BST
brilliant, thx konstantinous.
https://libre-soc.org/irclog/%23libre-soc.2022-07-26.log.html#t2022-07-26T10:18:01

minor correction needed (as discussed above)
https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=51ebd21c50d53275e8dd828a997433175b6dd2a2

+------00011,ALU,OP_FMVIS,FRS,CONST_UI,NONE,FRS,NONE,N
should have been out=NONE:
+------00011,ALU,OP_FMVIS,FRS,CONST_UI,NONE,NONE,NONE,N

unlike frsli which *is* a read-modify-write.
Comment 7 Konstantinos Margaritis (markos) 2022-07-27 12:04:19 BST
Both instructions should be completed in commit 04f20aa.
Comment 8 Luke Kenneth Casson Leighton 2022-07-27 14:32:47 BST
1.6.1.6 DX-FORM
0  6  11 16 26 31 
PO RT d1 d0 XO d2
Figure 8. DX instruction format

just checking PDF Power ISA v3.1 this does not match with
the changes you made.  check fields.text

https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=04f20aaff9ad03410dd7c6f483a50c754667f99c

     # V3.0B 1.6.6 DX-FORM
     # |0     |6 |7|8|9  |10  |11|12|13  |15|16|17     |26|27    |31  |
-    # | PO   |   FRS         |     d1      |      d0     |   XO |d2  |
+    # | PO   |   FRS         |     d1      |      d0  |      XO |d2  |

that doesn't match the spec. something else is going on.
https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=openpower/isatables/fields.text;h=7037463cda1e0c4372aa1cb491e281039223f611;hb=d64bf3662989d5f04da0657e154ccea5124e2872

 51 # V3.0B 1.6.6 DX-FORM
  52     |0    |6   |11   |16   |26   |31
  53     | PO  |  RT|   d1|   d0|   XO|d2
  54     | PO  | FRS|   d1|   d0|   XO|d2

that matches the PDF.  don't for goodness sake modify fields.text

what the heck is going on.
Comment 9 Luke Kenneth Casson Leighton 2022-07-27 14:42:37 BST
fields.text:

 426     d0,d1,d2 (16:25,11:15,31)
 427         Immediate fields that are concatenated to specify a
 428         16-bit signed two's complement integer which is
 429         sign-extended to 64 bits.
 430         Formats: DX

Power ISA V3 1 spec

     d0,d1,d2 (16:25,11:15,31)


addpcis RT,D
 D ← d0||d1||d2
 
 213 * fmvis FRS,D
 214 
 215 Pseudo-code:
 216 
 217     bf16 <- d0 || d1 || d2
 218     fp32 <- bf16 || [0]*16
 219     FRS  <- DOUBLE(fp32)

yep that's all good, where the hell is it.
Comment 10 Luke Kenneth Casson Leighton 2022-07-27 14:46:17 BST
 347     return instruction(
 348         (PO , 0 , 5),
 349         (FRS, 6 , 10),
 350         (d1,  11, 15),
 351         (d0,  16, 25), # matches spec.
 352         (XO , 26, 30), # matches spec
 353         (d2 , 31, 31),
 354     )

yep, you are right.  i got -/+ inverted from the diff. err.

yep agreed all good.
Comment 11 Luke Kenneth Casson Leighton 2022-07-27 15:07:14 BST
all good, closing. confirmed unit tests functional.
spec updated.
Comment 12 Jacob Lifshay 2022-07-27 18:59:09 BST
actually, fishmv's pseudocode was broken, i added a test demonstrating that and fixed it:
https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=bc5f9cfaa3e3c25bbe11050f04d0556f114ded30
Comment 13 Jacob Lifshay 2022-07-27 19:06:26 BST
(In reply to Jacob Lifshay from comment #12)
> actually, fishmv's pseudocode was broken, i added a test demonstrating that
> and fixed it:
> https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;
> h=bc5f9cfaa3e3c25bbe11050f04d0556f114ded30

basically, there were no tests with the f32 lsb set, so you didn't notice the logic being messed up. also, your code was almost certainly broken for denormal values, since those need both RS's mantissa and the immediate to be variably shifted.
Comment 14 Luke Kenneth Casson Leighton 2022-07-28 21:34:26 BST
https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=9fc4f5fd4ec2e3a3e52acacaf699f18d324b9f2d

fmvis is in1=FRS,out=NONE (not in1=NONE,out=FRS)