Bug 1119

Summary: add byte reverse instructions in PowerISA v3.1B to ISACaller
Product: Libre-SOC's first SoC Reporter: Jacob Lifshay <programmerjake>
Component: Source CodeAssignee: Jacob Lifshay <programmerjake>
Status: RESOLVED FIXED    
Severity: enhancement CC: ghostmansd, libre-soc-bugs, lkcl, programmerjake
Priority: ---    
Version: unspecified   
Hardware: PC   
OS: Linux   
See Also: https://bugs.libre-soc.org/show_bug.cgi?id=1147
https://bugs.libre-soc.org/show_bug.cgi?id=1148
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:
Bug Depends on:    
Bug Blocks: 1120    

Description Jacob Lifshay 2023-07-20 02:05:33 BST
* DONE: brh, brw, brd
Comment 1 Jacob Lifshay 2023-07-20 02:11:08 BST
I implemented brh, brw, and brd and added a unit test. I needed to adjust insndb to use .long for some official instructions too, so I changed it to use the "unofficial" CSV column being "0" instead of "" to mean that it's an official instruction that needs .long.

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

https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=e3934f3f52583d4527afd059c3902a673d039ac2

commit e3934f3f52583d4527afd059c3902a673d039ac2
Author: Jacob Lifshay <programmerjake@gmail.com>
Date:   Wed Jul 19 18:00:58 2023 -0700

    add byte reverse instructions from PowerISA v3.1B

commit 16fd54a93b1db1ca97f0abf2ea5b685b7d090fa9
Author: Jacob Lifshay <programmerjake@gmail.com>
Date:   Wed Jul 19 17:59:32 2023 -0700

    support official instructions that need .long format
Comment 2 Jacob Lifshay 2023-07-22 02:52:26 BST
I forgot SVP64 tests here too:

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

commit 02d38e534f65e95d4efcbb130a28dd094c09ce84
Author: Jacob Lifshay <programmerjake@gmail.com>
Date:   Fri Jul 21 18:47:34 2023 -0700

    add SVP64 test for byte reverse insns
Comment 3 Dmitry Selyutin 2023-07-22 06:37:55 BST
Likely binutils need this too
Comment 4 Luke Kenneth Casson Leighton 2023-07-22 12:05:33 BST
(In reply to Dmitry Selyutin from comment #3)
> Likely binutils need this too

let's keep this open, there is plenty budget to do that.
Comment 5 Luke Kenneth Casson Leighton 2023-07-22 12:42:23 BST
jacob looks great: the patch to add the instruction is a good worked-example.
please do remember to keep to the project standard coding format.

* idx instead of really_long_unnecessary_variable_name
* "%s %d" % (mnemonic, idx) instead of PHP-style {mnemonic} {idx}
* x = y+5
  z = x*3
  instead of
  x = (y+5)*3 as a way to create *readable* code that does not
  hit the vertical-challenged-limitations of autopep8
* itertools.product() as a way to keep the indentation down.

everything in this patch is specifically targetted at getting the
indentation levels down.  the code is exactly the same number of lines:
one is readable (containing regular vertical alignment), the other is not.

readable ==> "easy to review" which is an extremely important part of
project maintenance, and your role is to make it easy for me to fulfil
*my* role.

https://git.libre-soc.org/?p=openpower-isa.git;a=commitdiff;h=d6b041c7
Comment 6 Luke Kenneth Casson Leighton 2023-07-22 12:45:55 BST
(In reply to Luke Kenneth Casson Leighton from comment #5)

> indentation levels down.  the code is exactly the same number of lines:

surprisingly: ~10% less despite having additional temporary variables.
i really wasn't expecting that.
Comment 7 Dmitry Selyutin 2023-08-30 16:17:00 BST
I think the budget for me should be moved to another task (#1148). I'll handle this.
Comment 8 Jacob Lifshay 2023-08-30 17:33:23 BST
this task is now a subtask of 1120
binutils moved to bug #1147