Bug 950 - pysvp64asm: support insndb-based assembly for SVP64 instructions
Summary: pysvp64asm: support insndb-based assembly for SVP64 instructions
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: Dmitry Selyutin
URL:
Depends on:
Blocks:
 
Reported: 2022-10-13 17:38 BST by Dmitry Selyutin
Modified: 2023-03-28 09:14 BST (History)
2 users (show)

See Also:
NLnet milestone: NLnet.2021-08-071.cavatools
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: 947
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 Dmitry Selyutin 2022-10-13 17:38:23 BST
As the last stage, once we support word instructions and various operands, we should be able to support SVP64 instructions. It's assumed that these will be represented via ".llong" statements, and followed the comments which describe the way the remapping was done.
Comment 1 Dmitry Selyutin 2022-11-11 20:37:51 GMT
Currently I implement support for specifiers; the recent commits introduce vec2/vec3/vec4 and w/sw/dw specifiers. I'm still thinking about how to support other specifiers in the best way possible, since the fields used differ on per-mode basis. Perhaps this will be easier to go from the disassembly in this regard, because svp64.py is somewhat obscure code-wise in the way it reuses same bits between different modes.
Comment 2 Luke Kenneth Casson Leighton 2022-11-11 22:32:56 GMT
(In reply to Dmitry Selyutin from comment #1)

> disassembly in this regard, because svp64.py is somewhat obscure code-wise
> in the way it reuses same bits between different modes.

yes i thought it would be a good idea (at the time) to merge similar modes.
before making any significant changes can i recommend adding a comprehensive
set of examples to test_pysvp64dis.py, because it will catch any errors:
the test does back-to-back assemble-disassemble.
Comment 3 Dmitry Selyutin 2022-11-13 21:27:28 GMT
(In reply to Luke Kenneth Casson Leighton from comment #2)
> 
> yes i thought it would be a good idea (at the time) to merge similar modes.
> before making any significant changes

Actually I don't almost touch pyssvp64asm, these two ways of assembly co-exist for now: the choice of backend is done via INSNDB environment variable. Once I feel I covered most of these, I'll proceed with testing and dropping the legacy. I think that eventually everything in assembly will be done by new backend, and we will only keep macros and "lexing" in pysvp64asm. I intentionally skip the testing for now, because almost everything is expected to fail at this point, since we don't have all specifiers supported.

> can i recommend adding a comprehensive
> set of examples to test_pysvp64dis.py, because it will catch any errors:
> the test does back-to-back assemble-disassemble.

Yes, this is one of the goals ultimately, it's just that I have not that much to test yet, since most of disassembly tests use specifiers extensively. But, once these are ready, I will populate them.
Comment 4 Dmitry Selyutin 2022-11-13 21:31:16 GMT
I've started support for ff/pr/m/sm/dm. These are complicated, for now I've almost done ff/pr chunks; I'll likely split that huge specifier class into two or will introduce some base class, since all of these rely on predicates. Below is an example of how it works (there's an error in binutils, ignore it for now, I'll fix it later).

$ cat /tmp/test.s && INSNDB=true SILENCELOG=true pysvp64asm /tmp/test.s /tmp/py.s && echo "### PYSVP64ASM ###" && cat /tmp/py.s && echo "### BINUTILS ###" && powerpc64le-linux-gnu-as -mlibresoc /tmp/py.s -o /tmp/test.o && powerpc64le-linux-gnu-objcopy -Obinary /tmp/test.o /tmp/bin.o && powerpc64le-linux-gnu-objdump -dr -Mlibresoc /tmp/test.o && echo "### PYSVP64DIS ###" && time pysvp64dis /tmp/bin.o 
sv.add./dw=32/sw=8/ff=eq *122,56,*3
sv.add./dw=32/sw=8/ff=un *122,56,*3
sv.add/dw=32/sw=8/ff=RC1 *122,56,*3
sv.add/dw=32/sw=8/ff=~RC1 *122,56,*3
### PYSVP64ASM ###
.long 0x054731EA; .long 0x7FD80215; # add./dw=32/sw=8/ff=eq *122,56,*3 # sv.add./dw=32/sw=8/ff=eq *122,56,*3
.long 0x054731EB; .long 0x7FD80215; # add./dw=32/sw=8/ff=un *122,56,*3 # sv.add./dw=32/sw=8/ff=un *122,56,*3
.long 0x054731E9; .long 0x7FD80214; # add/dw=32/sw=8/ff=RC1 *122,56,*3 # sv.add/dw=32/sw=8/ff=RC1 *122,56,*3
.long 0x054731ED; .long 0x7FD80214; # add/dw=32/sw=8/ff=~RC1 *122,56,*3 # sv.add/dw=32/sw=8/ff=~RC1 *122,56,*3
### BINUTILS ###

/tmp/test.o:     file format elf64-powerpcle


Disassembly of section .text:

0000000000000000 <.text>:
   0:   ea 31 47 05     sv.add./ff=eq/dw=32/sw=8 *r122,r56,*r3
   4:   15 02 d8 7f 
   8:   eb 31 47 05     sv.add./ff=so/dw=32/sw=8 *r122,r56,*r3
   c:   15 02 d8 7f 
  10:   e9 31 47 05     sv.add/ff=gt/dw=32/sw=8 *r122,r56,*r3
  14:   14 02 d8 7f 
  18:   ed 31 47 05     sv.add/ff=le/dw=32/sw=8 *r122,r56,*r3
  1c:   14 02 d8 7f 
### PYSVP64DIS ###
ea 31 47 05    sv.add./dw=32/ff=eq/sw=8 *r122,r56,*r3
15 02 d8 7f    
eb 31 47 05    sv.add./dw=32/ff=so/sw=8 *r122,r56,*r3
15 02 d8 7f    
e9 31 47 05    sv.add/dw=32/ff=RC1/sw=8 *r122,r56,*r3
14 02 d8 7f    
ed 31 47 05    sv.add/dw=32/ff=~RC1/sw=8 *r122,r56,*r3
Comment 5 Dmitry Selyutin 2022-11-13 21:34:15 GMT
Note that this is capable of error handling, and, thanks to specifier classes, this can be somewhat more flexible. And, well, SpecifierPredicate should be split into SpecifierFF, SpecifierPR, SpecifierMask classes, with some possible parent classes; it's dirty for now to have at least something to work with. Once these are split, the code may take a real advantage of this, and introduce better error messages and re-use the code in a proper way.
Comment 6 Dmitry Selyutin 2022-11-13 21:36:48 GMT
I'm not sure of regexes, too. They are neat in a sense that one can see something like BNF, but I think eventually these will be changed with class methods. Not the last reason is that it'd be simpler to switch binutils to the same form (after all, they don't use regexes in binutils, eh?).
Comment 7 Dmitry Selyutin 2022-11-14 21:37:08 GMT
It took longer than I expected, but still I've refactored the specifiers classes.

https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=faaf7caa57c72cbfd2226e5048265d88af2134d0
https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=70e4024ee40fa80e4e9b41a088b10774a58a8706
https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=de1763f20c9a238b8527f09b06abd54a62be9d2e
https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=e7a857ee379c3a93bfb81bb6d4d144807f07f35e

I dropped regexes, and introduced yet another enumeration for all possible predicates, and _yet_ another one to determine the classes of predicates (INTEGER/CR/RC1).
We already have 38 enumerations in only power_enums, and I don't intend to stop! :-D
Comment 8 Dmitry Selyutin 2022-11-14 21:39:19 GMT
Frankly we could've had enum per each m/sm/dm and w/sw/dw, but hey, I'm too lazy to do it. Perhaps at some point my enum obsession will eventually lead us to hundreds of enumerations...
Comment 9 Dmitry Selyutin 2022-11-14 21:44:51 GMT
Also, I think that we should rename ffrc1/ffrc0/prrc1/prrc0 and ff3/ff5 to any of the following:

FFRc1/FFRc0/PRRc1/PRRc0 and FF3bit/FF5bit
ff1/ff0/pr1/pr0 and ff3/ff5

The first form seems to be clearer, since it better expresses the intention. Opinions?
Comment 10 Luke Kenneth Casson Leighton 2022-11-14 22:37:43 GMT
(In reply to Dmitry Selyutin from comment #9)

> The first form seems to be clearer, since it better expresses the intention.
> Opinions?

if it's internal and it's clear, i have no problem at all.
Comment 11 Dmitry Selyutin 2022-11-22 16:12:34 GMT
I think I supported all specifiers except for pi (which seems to be the recent addition and I'm not sure whether it's ready or stable). As I mentioned, in the future, this code should be generated from some description we don't have yet, or, well, at least we should strive for generating it (I'm not sure if every bit of logic can be expressed with some descriptive unless we introduce kind of a toy language).

The next steps are:
1. Check that the assembly works with tests (it's a quite unlikely scenario that things will work immediately).
2. Clean the code up: for example, some dataclasses should be removed (definitely not those which come from the "static" configurations).
Comment 12 Dmitry Selyutin 2022-12-12 19:44:38 GMT
With few recent updates, insndb is able to complete test_caller.py and test_caller_bcd.py, much to my surprise. Checking the rest.
Comment 13 Dmitry Selyutin 2022-12-12 22:36:46 GMT
I cannot confirm it yet, but I have a wild guess that some tests might fail due to the fact that instructions are now fully decoded. Can it be the case? I compiled some code and it's the same in both master and insndb branch if compiled; the only thing different is the textual representation. Let's consider this code:

setvl 0, 0, 5, 1, 1, 1
sv.cmp 0, 1, *4, 14
sv.isel 14,*4,14,1
svstep. 12, 6, 0
sv.isel 10,12,10,1
svstep. 0, 1, 0
bc 6, 3, -0x24

With master branch, it gets converted to this:

.long 0x580009F6 # setvl 0, 0, 5, 1, 1, 1 # setvl 0, 0, 5, 1, 1, 1
.long 0x05400400; cmp 0, 1, 1, 14 # sv.cmp 0, 1, *4, 14
.long 0x05400800; isel 14, 1, 14, 1 # sv.isel 14,*4,14,1
.long 0x59800A27 # svstep. 12, 6, 0 # svstep. 12, 6, 0
.long 0x05400000; isel 10, 12, 10, 1 # sv.isel 10,12,10,1
.long 0x58000027 # svstep. 0, 1, 0 # svstep. 0, 1, 0
bc 6, 3, -0x24


With insndb branch, it gets converted to this:

.long 0x580009F6 # setvl 0,0,5,1,1,1 # setvl 0, 0, 5, 1, 1, 1
.long 0x05400400; .long 0x7C217000; # cmp 0,1,*4,14 # sv.cmp 0, 1, *4, 14
.long 0x05400800; .long 0x7DC1705E; # isel 14,*4,14,1 # sv.isel 14,*4,14,1
.long 0x59800A27 # svstep. 12,6,0 # svstep. 12, 6, 0
.long 0x05400000; .long 0x7D4C505E; # isel 10,12,10,1 # sv.isel 10,12,10,1
.long 0x58000027 # svstep. 0,1,0 # svstep. 0, 1, 0
.long 0x40C3FF70 # bc 6,3,-0x24 # bc 6, 3, -0x24

This should behave in an identical way; however, I still observe failures with src/openpower/decoder/isa/test_caller_setvl.py. Can my guess be correct? The reproducer is $(INSNDB=true SILENCELOG=true python3 src/openpower/decoder/isa/test_caller_setvl.py).
Comment 14 Luke Kenneth Casson Leighton 2022-12-13 09:52:06 GMT
(In reply to Dmitry Selyutin from comment #13)

> 
> With master branch, it gets converted to this:

> .long 0x580009F6 # setvl 0, 0, 5, 1, 1, 1 # setvl 0, 0, 5, 1, 1, 1
> .long 0x05400400; cmp 0, 1, 1, 14 # sv.cmp 0, 1, *4, 14
> .long 0x05400800; isel 14, 1, 14, 1 # sv.isel 14,*4,14,1
> .long 0x59800A27 # svstep. 12, 6, 0 # svstep. 12, 6, 0
> .long 0x05400000; isel 10, 12, 10, 1 # sv.isel 10,12,10,1
> .long 0x58000027 # svstep. 0, 1, 0 # svstep. 0, 1, 0
> bc 6, 3, -0x24

yep. notice how the scalar ops are preserved.  this is critical.

> 
> With insndb branch, it gets converted to this:
> 
> .long 0x580009F6 # setvl 0,0,5,1,1,1 # setvl 0, 0, 5, 1, 1, 1
> .long 0x05400400; .long 0x7C217000; # cmp 0,1,*4,14 # sv.cmp 0, 1, *4, 14

base instruction is needed here

> .long 0x05400800; .long 0x7DC1705E; # isel 14,*4,14,1 # sv.isel 14,*4,14,1

and here

> .long 0x59800A27 # svstep. 12,6,0 # svstep. 12, 6, 0
> .long 0x05400000; .long 0x7D4C505E; # isel 10,12,10,1 # sv.isel 10,12,10,1

and here

> .long 0x58000027 # svstep. 0,1,0 # svstep. 0, 1, 0
> .long 0x40C3FF70 # bc 6,3,-0x24 # bc 6, 3, -0x24

and here.

> 
> This should behave in an identical way; 

no it should not.  ISACaller is quite dumb: the assembly listing
has to be passed through to the unit test alongside the binary.
both are converted to lists and zipped up.
except for *explicitly added* instructions NOT found in ppc64le-gnu-
8 9 and 10 ISACaller *requires* the original assembler listing in
order to know and report the instruction.

on encountering ".long 0x40C3FF70" ISACaller is going, "what instruction
assembler mnemonic is that, i have no idea.

it should be reasonably straightforward to fix that but obviously
only when full decode back to assembler is also implemented.
Comment 15 Dmitry Selyutin 2023-01-15 15:54:21 GMT
There are 4 failures so far in disassembler test when the new assembly is used.

1. test_4_sv_crand: I have no idea how to encode `sv.crand/ff=XX`. What I see in pysvp64asm contradicts to https://libre-soc.org/openpower/sv/cr_ops/ page. Please visit https://libre-soc.org/irclog/%23libre-soc.2023-01-15.log.html#t2023-01-15T14:54:30 to check. Please update the fields to reflect the `mode |= failfirst << BO_MSB` combination. But I suspect all fields in the specs must be revisited. There's no RC1 field at all in the spec at CR ops.

2. test_20_cmp fails at `sv.cmp/dz/ff=RC1/m=r3/sz *4,1,*0,1`. It doesn't look like we're able to use dst-zero at either ff or pr modes.
https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/sv/trans/svp64.py;h=c61e92833d696d46c50bf715c898b33a3100aa49;hb=refs/heads/master#l1416
https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/sv/trans/svp64.py;h=c61e92833d696d46c50bf715c898b33a3100aa49;hb=refs/heads/master#l1456

3. test_16_bc fails at `sv.bc` instructions. These are likely mismatched due to the fact that they are not usual entries. We generally keep unprefixed instructions in both markdown and CSVs; `bc`, however, is a different beast. Is it possible to make it follow the well-known convention? If not, I'll have to add some hack into lookup, say, check `sv.XX` explicitly and, if `sv.XX` is found in markdown, emit these as separate records. I'd rather prefer to follow the standard convention, though.

4. test_15_predicates fails on `sv.extsw/dm=eq/sm=gt 3,7` instruction. According to pysvp64asm, it should have failed before: extsw is P2, and we cannot have different twin predicates.
https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/sv/trans/svp64.py;h=c61e92833d696d46c50bf715c898b33a3100aa49;hb=refs/heads/master#l1285
Comment 16 Dmitry Selyutin 2023-01-15 16:04:05 GMT
Except for 4 failures mentioned above, the disassembly test works. We, however, no longer output SVP64 as combination of "prefix plus backward-compatible instruction", and output a pair of longs. Instead of this, we act (almost) as a real assembler. For example, `sv.add./m=r3 *3,*7,*11` will be converted as shown below.

.long 0x05603FE0; .long 0x7C011215; # add./m=r3 *3,*7,*11 # sv.add./m=r3 *3,*7,*11

If that backward compatibility is essential (why? binutils don't need it), I'll have to output prefix as bytes and then disassemble the suffix to get its representation. Doable, but I'm not sure whether desirable.
Comment 17 Dmitry Selyutin 2023-01-15 16:25:50 GMT
Ah, and the reproducer for now is: `INSNDB=true SILENCELOG=true python3 ./src/openpower/sv/trans/test_pysvp64dis.py`. I haven't entirely dropped the old backend yet (though I doubt it will work, so please checkout the master branch to be sure).
Comment 18 Dmitry Selyutin 2023-01-17 18:33:04 GMT
(In reply to Dmitry Selyutin from comment #15)
> There are 4 failures so far in disassembler test when the new assembly is
> used.
>
> 4. test_15_predicates fails on `sv.extsw/dm=eq/sm=gt 3,7` instruction.
> According to pysvp64asm, it should have failed before: extsw is P2, and we
> cannot have different twin predicates.
> https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/sv/
> trans/svp64.py;h=c61e92833d696d46c50bf715c898b33a3100aa49;hb=refs/heads/
> master#l1285

Almost fixed, I found the issue.

(In reply to Dmitry Selyutin from comment #16)
> 
> If that backward compatibility is essential

For now I've introduced a backward compatibility layer for disassembly (called Style.LEGACY for obvious reasons). svp64dis is now also capable of this.

I managed to get some tests working, including svstep. The rest is in progress.
Comment 19 Dmitry Selyutin 2023-03-28 09:14:05 BST
As parent task is completed, I'm marking this as resolved.