Bug 1093 - Fix "GPR-or-zero" in binutils and openpower-isa
Summary: Fix "GPR-or-zero" in binutils and openpower-isa
Status: RESOLVED FIXED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Source Code (show other bugs)
Version: unspecified
Hardware: PC Windows
: --- enhancement
Assignee: Dmitry Selyutin
URL: https://libre-soc.org/irclog/%23libre...
Depends on: 1020 1098
Blocks: 952 961 1089
  Show dependency treegraph
 
Reported: 2023-05-29 17:21 BST by Dmitry Selyutin
Modified: 2023-06-03 19:19 BST (History)
2 users (show)

See Also:
NLnet milestone: NLnet.2022-08-051.OPF
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: 1020
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 2023-05-29 17:21:39 BST
Currently our support for RA_OR_ZERO and RT_OR_ZERO leaves much to be desired. These, when 0 is supplied as an operand, should yield 0, not r0, as usual RA and RT operands do.
1. openpower-isa disassembly doesn't diagnose these correctly at all, and treats them as usual GPRs.
2. binutils lack RT0 operand, the only instruction using RT0 is setvl, and support for this operand has been missing.

More details in IRC: https://libre-soc.org/irclog/%23libre-soc.2023-05-29.log.html#t2023-05-29T16:37:17.
Comment 1 Luke Kenneth Casson Leighton 2023-05-29 19:19:01 BST
apologies, this is just something that "fell through the gaps".
Comment 2 Dmitry Selyutin 2023-05-31 18:41:36 BST
This task has really evolved. Below is the list of instructions which don't pass the check[0] for different extras:

rlwimi
rlwimi.
fishmv
rldimi
rldimi.

[0] https://git.libre-soc.org/?p=openpower-isa.git;a=blob;f=src/openpower/decoder/power_insn.py;h=41a0c88e88f7dbeb21ef8df15b2e2205e20b196f;hb=refs/heads/master#l1269
Comment 3 Dmitry Selyutin 2023-05-31 18:45:41 BST
This error doesn't stops this task, it's just that this task helped to find this problem.
Comment 4 Dmitry Selyutin 2023-06-01 09:28:50 BST
I had to refactor operands and extras relationship, but now it works. No new failures in master for test_pysvp64dis, but I'll wait for CI.
17 new commits in extras branch (some are minor, though, just a style): https://git.libre-soc.org/?p=openpower-isa.git;a=shortlog;h=refs/heads/extras.

Luke, this task definitely deserves a budget, but I have no idea where to take it from. Could you take a look, please?
Comment 5 Dmitry Selyutin 2023-06-01 19:38:35 BST
No new failures on extras branch compared to the master.

extras: https://salsa.debian.org/Kazan-team/mirrors/openpower-isa/-/jobs/4265682
master: https://salsa.debian.org/Kazan-team/mirrors/openpower-isa/-/jobs/4265458
Comment 6 Dmitry Selyutin 2023-06-01 19:39:41 BST
The implementation is pushed into master. The rebase didn't cause any conflicts.
Comment 7 Luke Kenneth Casson Leighton 2023-06-03 16:17:24 BST
(In reply to Dmitry Selyutin from comment #3)
> This error doesn't stops this task, it's just that this task helped to find
> this problem.

ok i have it.

* there is no specifying "None" in the 4 Indexed areas
* therefore the value "0" is being detected/treated as "valid"
* but it contains corrupted data

        elif insn_name == 'rlwinm':
            # weird one, RA is a dest but not in bits 6:10
            res['0'] = 'd:RA;d:CR0'  # RA: Rdest1_EXTRA3
            res['1'] = 's:RS'  # RS: Rsrc1_EXTRA3

that's valid.

* *only* position Idx0 is valid
* *only* position Idx1 is valid

positions Idx2 and Idx3 are *not* valid and i think what is happening
is that "because not zero equals active":

line 1227

                if spec != 0:
                    vector = bool(spec[0])