[PATCH] disas/mips: Fix branch displacement for BEQZC and BNEZC

Philippe Mathieu-Daudé posted 1 patch 1 year, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20221013145246.4922-1-philmd@fungible.com
Maintainers: "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
There is a newer version of this series
disas/mips.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
[PATCH] disas/mips: Fix branch displacement for BEQZC and BNEZC
Posted by Philippe Mathieu-Daudé 1 year, 7 months ago
From: David Daney <david.daney@fungible.com>

disas/mips.c got added in commit 6643d27ea0 ("MIPS disas support")
apparently based on binutils tag 'gdb_6_1-branchpoint' [1].
Back then, MIPSr6 was not supported (added in binutils commit
7361da2c952 during 2014 [2]).

Binutils codebase diverged so much over the last 18 years, it is
not possible to simply cherry-pick their changes, so fix it BEQZC /
BNEZC 21-bit signed branch displacement locally.

[1] https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=opcodes/mips-dis.c;hb=refs/tags/gdb_6_1-branchpoint
[2] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=7361da2c952

Fixes: 31837be3ee ("target-mips: add compact and CP1 branches")
Signed-off-by: David Daney <david.daney@fungible.com>
Reviewed-by: Marcin Nowakowski <marcin.nowakowski@fungible.com>
[PMD: Added commit description]
Signed-off-by: Philippe Mathieu-Daudé <philmd@fungible.com>
---
 disas/mips.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/disas/mips.c b/disas/mips.c
index b9a5204304..68640b96d2 100644
--- a/disas/mips.c
+++ b/disas/mips.c
@@ -1334,9 +1334,9 @@ const struct mips_opcode mips_builtin_opcodes[] =
 {"balc",    "+p",       0xe8000000, 0xfc000000, UBD|WR_31,            0, I32R6},
 {"bc",      "+p",       0xc8000000, 0xfc000000, UBD|WR_31,            0, I32R6},
 {"jic",     "t,o",      0xd8000000, 0xffe00000, UBD|RD_t,             0, I32R6},
-{"beqzc",   "s,+p",     0xd8000000, 0xfc000000, CBD|RD_s,             0, I32R6},
+{"beqzc",   "s,+q",     0xd8000000, 0xfc000000, CBD|RD_s,             0, I32R6},
 {"jialc",   "t,o",      0xf8000000, 0xffe00000, UBD|RD_t,             0, I32R6},
-{"bnezc",   "s,+p",     0xf8000000, 0xfc000000, CBD|RD_s,             0, I32R6},
+{"bnezc",   "s,+q",     0xf8000000, 0xfc000000, CBD|RD_s,             0, I32R6},
 {"beqzalc", "s,t,p",    0x20000000, 0xffe00000, CBD|RD_s|RD_t,        0, I32R6},
 {"bovc",    "s,t,p",    0x20000000, 0xfc000000, CBD|RD_s|RD_t,        0, I32R6},
 {"beqc",    "s,t,p",    0x20000000, 0xfc000000, CBD|RD_s|RD_t,        0, I32R6},
@@ -4462,6 +4462,16 @@ print_insn_args (const char *d,
                 (*info->print_address_func) (info->target, info);
                 break;
 
+            case 'q':
+                /* Sign extend the displacement with 21 bits.  */
+                delta = l & 0x1FFFFF;
+                if (delta & 0x100000) {
+                    delta |= ~0x1FFFFF;
+                }
+                info->target = (delta << 2) + pc + INSNLEN;
+                (*info->print_address_func) (info->target, info);
+                break;
+
 	    case 't': /* Coprocessor 0 reg name */
 	      (*info->fprintf_func) (info->stream, "%s",
 				     mips_cp0_names[(l >> OP_SH_RT) &
-- 
2.37.3


Re: [PATCH] disas/mips: Fix branch displacement for BEQZC and BNEZC
Posted by Richard Henderson 1 year, 7 months ago
On 10/14/22 07:52, Philippe Mathieu-Daudé wrote:
> +                /* Sign extend the displacement with 21 bits.  */
> +                delta = l & 0x1FFFFF;
> +                if (delta & 0x100000) {
> +                    delta |= ~0x1FFFFF;
> +                }

delta = sextract32(l, 0, 21);

r~

Re: [PATCH] disas/mips: Fix branch displacement for BEQZC and BNEZC
Posted by Philippe Mathieu-Daudé 1 year, 7 months ago
On Thu, Oct 13, 2022 at 9:26 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 10/14/22 07:52, Philippe Mathieu-Daudé wrote:
> > +                /* Sign extend the displacement with 21 bits.  */
> > +                delta = l & 0x1FFFFF;
> > +                if (delta & 0x100000) {
> > +                    delta |= ~0x1FFFFF;
> > +                }

Note this follows the style of this file, ...

> delta = sextract32(l, 0, 21);

... but I agree using sextract() makes it easier to review. I'll respin.