Re: [PATCH^3] gdb: mips: Add MIPSR6 support

[Milos Kalicanin <Milos.Kalicanin@Syrmia.com>]

Thanks for reply, Simon. Your suggestions will be applied soon.

Speaking of test mechanic:
It's quite simple. Pseudocode follows:
-put breakpoint on each test function (test_r6_r6, test_r6, test_r6_llsc_dp...)
-single step using gdb command 'stepi', checking response:
        if (response == gdb_prompt) or (response == 'breakpoint hit'):
                proceed further
        elseif (response == exited abnormally)
                test failure
        elseif (response == exited normally)
                tests passed successfully
        else
                tests failure
-previous loop is constrained with timeout

Reason for 'abnormal exit' would be 'test function returned nonzero status':
> /*
> * Any test_r6_* function returns non-zero => failure
> */
> #define EXPECT(X) if ((X)) abort ();

In inline asm, register $11 or [err] identifies instruction currently being tested.
If any instruction's output is wrong, program jumps to epilog saving non-zero exit status and signaling failure. If all instructions are tested and returned expected result, output register [err] is set to 0, signaling success.

Let me know if you have any more suggestions/questions.

Kind Regards,
Milos

________________________________________
From: Simon Marchi <simark@simark.ca>
Sent: Friday, February 2, 2024 9:57 PM
To: Milos Kalicanin; gdb-patches@sourceware.org
Cc: Andrew Burgess; Maciej W . Rozycki; Chao-ying Fu; Djordje Todorovic; Dragan Mladjenovic; simon.marchi@sourceware.org
Subject: Re: [PATCH^3] gdb: mips: Add MIPSR6 support

On 1/30/24 10:17, Milos Kalicanin wrote:
> Introduce new instruction encodings from Release 6 of the MIPS
> architecture [1]. Support breakpoints and single stepping with
> compact branches, forbidden slots, new branch instruction and
> new atomic load-store instruction encodings.
>
> Changes from v2: Added new tests

Thanks for the update.

I noted a few cosmetic comments below.  As I said last time, I can only
do a superficial review.

> [1] "MIPS64 Architecture for Programmers Volume II-A: The MIPS64
>     Instruction Set Reference Manual", Document Number: MD00087,
>     Revision 6.06, December 15, 2016, Section 3 "The MIPS64
>     Instruction Set", pp. 42-530
> https://s3-eu-west-1.amazonaws.com/downloads-mips/documents/MD00087-2B-MIPS64BIS-AFP-6.06.pdf
>
> 2024-01-30  Andrew Bennett  <andrew.bennett@imgtec.com>
>           Matthew Fortune  <matthew.fortune@mips.com>
>           Faraz Shahbazker  <fshahbazker@wavecomp.com>
>
> gdb/ChangeLog:
>       * mips-tdep.c (is_mipsr6_isa): New.
>       (b0s21_imm): New define.
>       (mips32_relative_offset21, mips32_relative_offset26): New.
>       (is_add32bit_overflow, is_add64bit_overflow): New.
>       (mips32_next_pc): Handle r6 compact and fpu coprocessor branches.
>       Move handling of BLEZ, BGTZ opcode into ...
>       (mips32_blez_pc): New.
>       (mips32_instruction_is_compact_branch): New.
>       (mips32_insn_at_pc_has_forbidden_slot):  New.
>       (mips32_scan_prologue): Ignore pre-r6 addi encoding on r6.
>       Stop at compact branch also.
>       (LLSC_R6_OPCODE,LL_R6_FUNCT,LLE_FUNCT,
>       LLD_R6_FUNCT,SC_R6_FUNCT,SCE_FUNCT,
>       SCD_R6_FUNCT: New defines.
>       (is_ll_insn, is_sc_insn): New.
>       (mips_deal_with_atomic_sequence): Use is_ll_insn/is_sc_insn.
>       Handle compact branches.
>       (mips_about_to_return): Handle jrc and macro jr.
>       (mips32_stack_frame_destroyed_p): Likewise.
>       (mips32_instruction_has_delay_slot): Don't handle JALX on r6.
>       Handle compact branches and coprocessor branches.
>       (mips_adjust_breakpoint_address): Skip forbidden slot for
>       compact branches.
> ---
>  gdb/mips-tdep.c                       |  520 ++++++++-
>  gdb/testsuite/gdb.arch/mips-64-r6.c   | 1469 +++++++++++++++++++++++++
>  gdb/testsuite/gdb.arch/mips-64-r6.exp |   99 ++
>  3 files changed, 2046 insertions(+), 42 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.arch/mips-64-r6.c
>  create mode 100644 gdb/testsuite/gdb.arch/mips-64-r6.exp
>
> diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
> index bf0b66c4b00..a60377f25e0 100644
> --- a/gdb/mips-tdep.c
> +++ b/gdb/mips-tdep.c
> @@ -76,6 +76,9 @@ static int mips16_insn_at_pc_has_delay_slot (struct gdbarch *gdbarch,
>  static void mips_print_float_info (struct gdbarch *, struct ui_file *,
>                                  frame_info_ptr, const char *);
>
> +static void mips_read_fp_register_single (struct frame_info_ptr, int,
> +                                       gdb_byte *);
> +
>  /* A useful bit in the CP0 status register (MIPS_PS_REGNUM).  */
>  /* This bit is set if we are emulating 32-bit FPRs on a 64-bit chip.  */
>  #define ST0_FR (1 << 26)
> @@ -1500,6 +1503,16 @@ mips_fetch_instruction (struct gdbarch *gdbarch,
>    return extract_unsigned_integer (buf, instlen, byte_order);
>  }
>
> +/* Return one if the gdbarch is based on MIPS Release 6.  */
> +static int
> +is_mipsr6_isa (struct gdbarch *gdbarch)

Change to bool and "Return true".

> +{
> +  const struct bfd_arch_info *info = gdbarch_bfd_arch_info (gdbarch);
> +
> +  return (info->mach == bfd_mach_mipsisa32r6
> +       || info->mach == bfd_mach_mipsisa64r6);
> +}
> +
>  /* These are the fields of 32 bit mips instructions.  */
>  #define mips32_op(x) (x >> 26)
>  #define itype_op(x) (x >> 26)
> @@ -1542,6 +1555,7 @@ mips_fetch_instruction (struct gdbarch *gdbarch,
>  #define b0s11_op(x) ((x) & 0x7ff)
>  #define b0s12_imm(x) ((x) & 0xfff)
>  #define b0s16_imm(x) ((x) & 0xffff)
> +#define b0s21_imm(x) ((x) & 0x1fffff)
>  #define b0s26_imm(x) ((x) & 0x3ffffff)
>  #define b6s10_ext(x) (((x) >> 6) & 0x3ff)
>  #define b11s5_reg(x) (((x) >> 11) & 0x1f)
> @@ -1578,6 +1592,18 @@ mips32_relative_offset (ULONGEST inst)
>    return ((itype_immediate (inst) ^ 0x8000) - 0x8000) << 2;
>  }
>
> +static LONGEST
> +mips32_relative_offset21 (ULONGEST insn)
> +{
> +  return ((b0s21_imm (insn) ^ 0x100000) - 0x100000) << 2;
> +}
> +
> +static LONGEST
> +mips32_relative_offset26 (ULONGEST insn)
> +{
> +  return ((b0s26_imm (insn) ^ 0x2000000) - 0x2000000) << 2;
> +}
> +
>  /* Determine the address of the next instruction executed after the INST
>     floating condition branch instruction at PC.  COUNT specifies the
>     number of the floating condition bits tested by the branch.  */
> @@ -1636,6 +1662,71 @@ is_octeon_bbit_op (int op, struct gdbarch *gdbarch)
>    return 0;
>  }
>
> +static int
> +is_add32bit_overflow (int32_t a, int32_t b)

bool

> +{
> +  int32_t r = (uint32_t) a + (uint32_t) b;
> +  return (a < 0 && b < 0 && r >= 0) || (a >= 0 && b >= 0 && r < 0);
> +}
> +
> +static int
> +is_add64bit_overflow (int64_t a, int64_t b)

bool

> +{
> +  if (a != (int32_t)a)
> +    return 1;
> +  if (b != (int32_t)b)
> +    return 1;
> +  return is_add32bit_overflow ((int32_t)a, (int32_t)b);

Spaces after casts (4 times).

> +}
> +
> +/* Calculate address of next instruction after BLEZ.  */
> +
> +static CORE_ADDR
> +mips32_blez_pc (struct gdbarch *gdbarch, struct regcache *regcache,

Omit "struct" where possible.

> +             ULONGEST inst, CORE_ADDR pc, int invert)
> +{
> +  int rs = itype_rs (inst);
> +  int rt = itype_rt (inst);
> +  LONGEST val_rs = regcache_raw_get_signed (regcache, rs);
> +  LONGEST val_rt = regcache_raw_get_signed (regcache, rt);
> +  ULONGEST uval_rs = regcache_raw_get_unsigned (regcache, rs);
> +  ULONGEST uval_rt = regcache_raw_get_unsigned (regcache, rt);

Move declarations to more specific scopes when possible.

> +  int taken = 0;

bool and false

> +  int delay_slot_size = 4;
> +
> +  /* BLEZ, BLEZL, BGTZ, BGTZL */
> +  if (rt == 0)
> +    taken = (val_rs <= 0);
> +  else if (is_mipsr6_isa (gdbarch))
> +    {
> +      /* BLEZALC, BGTZALC */
> +      if (rs == 0 && rt != 0)
> +     taken = (val_rt <= 0);
> +      /* BGEZALC, BLTZALC */
> +      else if (rs == rt && rt != 0)
> +     taken = (val_rt >= 0);
> +      /* BGEUC, BLTUC */
> +      else if (rs != rt && rs != 0 && rt != 0)
> +     taken = (uval_rs >= uval_rt);
> +
> +      /* Step through the forbidden slot to avoid repeated exceptions we do
> +      not currently have access to the BD bit when hitting a breakpoint
> +      and therefore cannot tell if the breakpoint hit on the branch or the
> +      forbidden slot.  */
> +      /* delay_slot_size = 0; */
> +    }
> +
> +  if (invert)
> +    taken = !taken;
> +
> +  /* Calculate branch target.  */
> +  if (taken)
> +    pc += mips32_relative_offset (inst);
> +  else
> +    pc += delay_slot_size;
> +
> +  return pc;
> +}
>
>  /* Determine where to set a single step breakpoint while considering
>     branch prediction.  */
> @@ -1646,12 +1737,17 @@ mips32_next_pc (struct regcache *regcache, CORE_ADDR pc)
>    struct gdbarch *gdbarch = regcache->arch ();
>    unsigned long inst;
>    int op;
> +  int mips64bitreg = 0;
> +
> +  if (mips_isa_regsize (gdbarch) == 8)
> +    mips64bitreg = 1;

More simply:

  bool mips64bitreg = mips_isa_regsize (gdbarch) == 8);

> +
>    inst = mips_fetch_instruction (gdbarch, ISA_MIPS, pc, NULL);
>    op = itype_op (inst);
>    if ((inst & 0xe0000000) != 0)              /* Not a special, jump or branch
>                                          instruction.  */
>      {
> -      if (op >> 2 == 5)
> +      if (op >> 2 == 5 && ((op & 0x02) == 0 || itype_rt (inst) == 0))
>       /* BEQL, BNEL, BLEZL, BGTZL: bits 0101xx */
>       {
>         switch (op & 0x03)
> @@ -1661,7 +1757,7 @@ mips32_next_pc (struct regcache *regcache, CORE_ADDR pc)
>           case 1:             /* BNEL */
>             goto neq_branch;
>           case 2:             /* BLEZL */
> -           goto less_branch;
> +           goto lez_branch;
>           case 3:             /* BGTZL */
>             goto greater_branch;
>           default:
> @@ -1671,15 +1767,19 @@ mips32_next_pc (struct regcache *regcache, CORE_ADDR pc)
>        else if (op == 17 && itype_rs (inst) == 8)
>       /* BC1F, BC1FL, BC1T, BC1TL: 010001 01000 */
>       pc = mips32_bc1_pc (gdbarch, regcache, inst, pc + 4, 1);
> -      else if (op == 17 && itype_rs (inst) == 9
> +      else if (!is_mipsr6_isa (gdbarch)
> +            && op == 17
> +            && itype_rs (inst) == 9
>              && (itype_rt (inst) & 2) == 0)
>       /* BC1ANY2F, BC1ANY2T: 010001 01001 xxx0x */
>       pc = mips32_bc1_pc (gdbarch, regcache, inst, pc + 4, 2);
> -      else if (op == 17 && itype_rs (inst) == 10
> -            && (itype_rt (inst) & 2) == 0)
> +      else if (!is_mipsr6_isa (gdbarch)
> +             && op == 17
> +             && itype_rs (inst) == 10
> +             && (itype_rt (inst) & 2) == 0)
>       /* BC1ANY4F, BC1ANY4T: 010001 01010 xxx0x */
>       pc = mips32_bc1_pc (gdbarch, regcache, inst, pc + 4, 4);
> -      else if (op == 29)
> +      else if (!is_mipsr6_isa (gdbarch) && op == 29)
>       /* JALX: 011101 */
>       /* The new PC will be alternate mode.  */
>       {
> @@ -1707,7 +1807,128 @@ mips32_next_pc (struct regcache *regcache, CORE_ADDR pc)
>         else
>           pc += 8;        /* After the delay slot.  */
>       }
> +      else if (is_mipsr6_isa (gdbarch))
> +     {
> +       /* BOVC, BEQZALC, BEQC and BNVC, BNEZALC, BNEC */
> +       if (op == 8 || op == 24)
> +         {
> +           int rs = rtype_rs (inst);
> +           int rt = rtype_rt (inst);
> +           LONGEST val_rs = regcache_raw_get_signed (regcache, rs);
> +           LONGEST val_rt = regcache_raw_get_signed (regcache, rt);
> +           int taken = 0;

Add new line after this.  Also, use bool and false.

> +           /* BOVC (BNVC) */
> +           if (rs >= rt)
> +             {
> +               if (mips64bitreg == 1)
> +                 taken = is_add64bit_overflow (val_rs, val_rt);
> +               else
> +                 taken = is_add32bit_overflow (val_rs, val_rt);
> +             }
> +           /* BEQZALC (BNEZALC) */
> +           else if (rs < rt && rs == 0)
> +             taken = (val_rt == 0);
> +           /* BEQC (BNEC) */
> +           else
> +             taken = (val_rs == val_rt);
> +
> +           /* BNVC, BNEZALC, BNEC */
> +           if (op == 24)
> +             taken = !taken;
>
> +           if (taken)
> +             pc += mips32_relative_offset (inst) + 4;
> +           else
> +             /* Step through the forbidden slot to avoid repeated exceptions
> +                we do not currently have access to the BD bit when hitting a
> +                breakpoint and therefore cannot tell if the breakpoint
> +                hit on the branch or the forbidden slot.  */
> +             pc += 8;

Add { } braces to the previous else (because of the presence of the
comment).

> +         }
> +       /* BC1EQZ, BC1NEZ */
> +       else if (op == 17 && (itype_rs (inst) == 9 || itype_rs (inst) == 13))
> +         {
> +           gdb_byte status;
> +           gdb_byte true_val = 0;
> +           unsigned int fp = (gdbarch_num_regs (gdbarch)
> +                              + mips_regnum (gdbarch)->fp0
> +                              + itype_rt (inst));
> +           struct frame_info_ptr frame = get_current_frame ();
> +           gdb_byte *raw_buffer = (gdb_byte *) alloca (sizeof (gdb_byte) * 4);
> +           mips_read_fp_register_single (frame, fp, raw_buffer);
> +
> +           if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
> +             status = *(raw_buffer + 3);
> +           else
> +             status = *(raw_buffer);
> +
> +           if (itype_rs (inst) == 13)
> +             true_val = 1;
> +
> +           if ((status & 0x1) == true_val)
> +             pc += mips32_relative_offset (inst) + 4;
> +           else
> +             pc += 8;
> +         }
> +       else if (op == 22 || op == 23)
> +       /* BLEZC, BGEZC, BGEC, BGTZC, BLTZC, BLTC */
> +         {
> +           int rs = rtype_rs (inst);
> +           int rt = rtype_rt (inst);
> +           LONGEST val_rs = regcache_raw_get_signed (regcache, rs);
> +           LONGEST val_rt = regcache_raw_get_signed (regcache, rt);
> +           int taken = 0;
> +           /* The R5 rt == 0 case is handled above so we treat it as
> +              an unknown instruction here for future ISA usage.  */
> +           if (rs == 0 && rt != 0)
> +             taken = (val_rt <= 0);
> +           else if (rs == rt && rt != 0)
> +             taken = (val_rt >= 0);
> +           else if (rs != rt && rs != 0 && rt != 0)
> +             taken = (val_rs >= val_rt);
> +
> +           if (op == 23)
> +             taken = !taken;
> +
> +           if (taken)
> +             pc += mips32_relative_offset (inst) + 4;
> +           else
> +             /* Step through the forbidden slot to avoid repeated exceptions
> +                we do not currently have access to the BD bit when hitting a
> +                breakpoint and therefore cannot tell if the breakpoint
> +                hit on the branch or the forbidden slot.  */
> +             pc += 8;

Add curly braces.

> +         }
> +       else if (op == 50 || op == 58)
> +       /* BC, BALC */
> +         pc += mips32_relative_offset26 (inst) + 4;
> +       else if ((op == 54 || op == 62)
> +                && rtype_rs (inst) == 0)
> +       /* JIC, JIALC */
> +         {
> +           pc = regcache_raw_get_signed (regcache, itype_rt (inst));
> +           pc += (itype_immediate (inst) ^ 0x8000) - 0x8000;
> +         }
> +       else if (op == 54 || op == 62)
> +       /* BEQZC, BNEZC */
> +         {
> +           int rs = itype_rs (inst);
> +           LONGEST rs_val = regcache_raw_get_signed (regcache, rs);
> +           int taken = (rs_val == 0);
> +           if (op == 62)
> +             taken = !taken;
> +           if (taken)
> +             pc += mips32_relative_offset21 (inst) + 4;
> +           else
> +             /* Step through the forbidden slot to avoid repeated exceptions
> +                we do not currently have access to the BD bit when hitting a
> +                breakpoint and therefore cannot tell if the breakpoint
> +                hit on the branch or the forbidden slot.  */
> +             pc += 8;

Add curly braces.

> +         }
> +       else
> +         pc += 4;            /* Not a branch, next instruction is easy.  */
> +     }
>        else
>       pc += 4;                /* Not a branch, next instruction is easy.  */
>      }
> @@ -1751,7 +1972,6 @@ mips32_next_pc (struct regcache *regcache, CORE_ADDR pc)
>             case 2:           /* BLTZL */
>             case 16:          /* BLTZAL */
>             case 18:          /* BLTZALL */
> -           less_branch:
>               if (regcache_raw_get_signed (regcache, itype_rs (inst)) < 0)
>                 pc += mips32_relative_offset (inst) + 4;
>               else
> @@ -1767,22 +1987,38 @@ mips32_next_pc (struct regcache *regcache, CORE_ADDR pc)
>                 pc += 8;      /* after the delay slot */
>               break;
>             case 0x1c:        /* BPOSGE32 */
> +           case 0x1d:        /* BPOSGE32C */
>             case 0x1e:        /* BPOSGE64 */
>               pc += 4;
>               if (itype_rs (inst) == 0)
>                 {
>                   unsigned int pos = (op & 2) ? 64 : 32;
>                   int dspctl = mips_regnum (gdbarch)->dspctl;
> +                 int delay_slot_size = 4;
>
>                   if (dspctl == -1)
>                     /* No way to handle; it'll most likely trap anyway.  */
>                     break;
>
> +                 /* BPOSGE32C */
> +                 if (op == 0x1d)
> +                   {
> +                     if (!is_mipsr6_isa (gdbarch))
> +                       break;
> +
> +                     /* Step through the forbidden slot to avoid repeated
> +                        exceptions we do not currently have access to the BD
> +                        bit when hitting a breakpoint and therefore cannot
> +                        tell if the breakpoint hit on the branch or the
> +                        forbidden slot.  */
> +                     /* delay_slot_size = 0; */
> +                   }
> +
>                   if ((regcache_raw_get_unsigned (regcache,
>                                                   dspctl) & 0x7f) >= pos)
>                     pc += mips32_relative_offset (inst);
>                   else
> -                   pc += 4;
> +                   pc += delay_slot_size;
>                 }
>               break;
>               /* All of the other instructions in the REGIMM category */
> @@ -1816,19 +2052,14 @@ mips32_next_pc (struct regcache *regcache, CORE_ADDR pc)
>         else
>           pc += 8;
>         break;
> -     case 6:         /* BLEZ, BLEZL */
> -       if (regcache_raw_get_signed (regcache, itype_rs (inst)) <= 0)
> -         pc += mips32_relative_offset (inst) + 4;
> -       else
> -         pc += 8;
> +     case 6:         /* BLEZ, BLEZL, BLEZALC, BGEZALC, BGEUC */
> +     lez_branch:
> +       pc = mips32_blez_pc (gdbarch, regcache, inst, pc + 4, 0);
>         break;
>       case 7:
>       default:
> -     greater_branch: /* BGTZ, BGTZL */
> -       if (regcache_raw_get_signed (regcache, itype_rs (inst)) > 0)
> -         pc += mips32_relative_offset (inst) + 4;
> -       else
> -         pc += 8;
> +     greater_branch: /* BGTZ, BGTZL, BGTZALC, BLTZALC, BLTUC */
> +       pc = mips32_blez_pc (gdbarch, regcache, inst, pc + 4, 1);
>         break;
>       }                       /* switch */
>      }                                /* else */
> @@ -2451,6 +2682,72 @@ micromips_instruction_is_compact_branch (unsigned short insn)
>      }
>  }
>
> +/* Return non-zero if the MIPS instruction INSN is a compact branch
> +   or jump.  A value of 1 indicates an unconditional compact branch
> +   and a value of 2 indicates a conditional compact branch.  */
> +
> +static int
> +mips32_instruction_is_compact_branch (struct gdbarch *gdbarch, ULONGEST insn)

Can you please add an enum type for the result?

Also, the "is" in the function makes it sound like the function
determines if INSN is a compact branch or not, and that it would return
a boolean.  But the comment makes it sound like it classifies the
instruction in some categories.  Can you find a better name for the
function?

> +{
> +  switch (itype_op (insn))
> +    {
> +    /* BC */
> +    case 50:
> +    /* BALC */
> +    case 58:
> +      if (is_mipsr6_isa (gdbarch))
> +     return 1;
> +      break;
> +    /* BOVC, BEQZALC, BEQC */
> +    case 8:
> +    /* BNVC, BNEZALC, BNEC */
> +    case 24:
> +      if (is_mipsr6_isa (gdbarch))
> +     return 2;
> +      break;
> +    /* BEQZC, JIC */
> +    case 54:
> +    /* BNEZC, JIALC */
> +    case 62:
> +      if (is_mipsr6_isa (gdbarch))
> +     /* JIC, JIALC are unconditional */
> +     return (itype_rs (insn) == 0) ? 1 : 2;
> +      break;
> +    /* BLEZC, BGEZC, BGEC */
> +    case 22:
> +    /* BGTZC, BLTZC, BLTC */
> +    case 23:
> +    /* BLEZALC, BGEZALC, BGEUC */
> +    case 6:
> +    /* BGTZALC, BLTZALC, BLTUC */
> +    case 7:
> +      if (is_mipsr6_isa (gdbarch)
> +       && itype_rt (insn) != 0)
> +     return 2;
> +      break;
> +    /* BPOSGE32C */
> +    case 1:
> +      if (is_mipsr6_isa (gdbarch)
> +       && itype_rt (insn) == 0x1d && itype_rs (insn) == 0)
> +     return 2;
> +    }
> +  return 0;
> +}
> +
> +/* Return non-zero if a standard MIPS instruction at ADDR has a branch
> +   forbidden slot (i.e. it is a conditional compact branch instruction).  */
> +
> +static int
> +mips32_insn_at_pc_has_forbidden_slot (struct gdbarch *gdbarch, CORE_ADDR addr)

"Return true" and bool.

> +{
> +  int status;
> +  ULONGEST insn = mips_fetch_instruction (gdbarch, ISA_MIPS, addr, &status);
> +  if (status)
> +    return 0;
> +
> +  return mips32_instruction_is_compact_branch (gdbarch, insn) == 2;
> +}
> +
>  struct mips_frame_cache
>  {
>    CORE_ADDR base;
> @@ -3494,7 +3791,8 @@ mips32_scan_prologue (struct gdbarch *gdbarch,
>        reg = high_word & 0x1f;
>
>        if (high_word == 0x27bd                /* addiu $sp,$sp,-i */
> -       || high_word == 0x23bd        /* addi $sp,$sp,-i */
> +       || (high_word == 0x23bd       /* addi $sp,$sp,-i */
> +           && !is_mipsr6_isa (gdbarch))
>         || high_word == 0x67bd)       /* daddiu $sp,$sp,-i */
>       {
>         if (offset < 0)               /* Negative stack adjustment?  */
> @@ -3632,7 +3930,9 @@ mips32_scan_prologue (struct gdbarch *gdbarch,
>
>        /* A jump or branch, or enough non-prologue insns seen?  If so,
>        then we must have reached the end of the prologue by now.  */
> -      if (prev_delay_slot || non_prologue_insns > 1)
> +      if (prev_delay_slot
> +       || non_prologue_insns > 1
> +       || mips32_instruction_is_compact_branch (gdbarch, inst))
>       break;
>
>        prev_non_prologue_insn = this_non_prologue_insn;
> @@ -3938,6 +4238,59 @@ mips_addr_bits_remove (struct gdbarch *gdbarch, CORE_ADDR addr)
>  #define LLD_OPCODE 0x34
>  #define SC_OPCODE 0x38
>  #define SCD_OPCODE 0x3c
> +#define LLSC_R6_OPCODE 0x1f
> +#define LL_R6_FUNCT 0x36
> +#define LLE_FUNCT 0x2e
> +#define LLD_R6_FUNCT 0x37
> +#define SC_R6_FUNCT 0x26
> +#define SCE_FUNCT 0x1e
> +#define SCD_R6_FUNCT 0x27
> +
> +static int
> +is_ll_insn (struct gdbarch *gdbarch, ULONGEST insn)

bool, and true/false below.

> +{
> +  if (itype_op (insn) == LL_OPCODE
> +      || itype_op (insn) == LLD_OPCODE)
> +    return 1;
> +
> +  if (rtype_op (insn) == LLSC_R6_OPCODE
> +      && rtype_funct (insn) == LLE_FUNCT
> +      && (insn & 0x40) == 0)
> +    return 1;
> +
> +  /* Handle LL and LLP varieties.  */
> +  if (is_mipsr6_isa (gdbarch)
> +      && rtype_op (insn) == LLSC_R6_OPCODE
> +      && (rtype_funct (insn) == LL_R6_FUNCT
> +       || rtype_funct (insn) == LLD_R6_FUNCT
> +       || rtype_funct (insn) == LLE_FUNCT))
> +    return 1;
> +
> +  return 0;
> +}
> +
> +static int
> +is_sc_insn (struct gdbarch *gdbarch, ULONGEST insn)

Same.

> +}
> +
> +
> +
> +/*
> + * Any test_r6_* function returns non-zero => failure
> + */
> +#define EXPECT(X) if ((X)) abort ();
> +
> +
> +main ()
> +{
> +  EXPECT(test_r6_branch());
> +
> +  EXPECT(test_r6_forbidden());
> +
> +  EXPECT(test_r6_64());
> +
> +  EXPECT(test_r6());
> +
> +  EXPECT(test_r6_fpu());
> +
> +
> +  EXPECT(test_r6_llsc_dp());
> +
> +
> +  EXPECT(test_r6_llsc_wp());
> +}
> diff --git a/gdb/testsuite/gdb.arch/mips-64-r6.exp b/gdb/testsuite/gdb.arch/mips-64-r6.exp
> new file mode 100644
> index 00000000000..1ab04daddd5
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/mips-64-r6.exp
> @@ -0,0 +1,99 @@
> +# Copyright (C) 2012-2023 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, see <http://www.gnu.org/licenses/>.
> +
> +# Test mips release 6 patch.
> +
> +require {istarget "*mips*"}
> +
> +proc single_step {} {
> +    global gdb_prompt
> +
> +    send_gdb "si\n"
> +    gdb_expect {
> +     -re "$gdb_prompt \$" {
> +         return 1
> +     }
> +     -re ".*Breakpoint.*test_.*" {
> +         return 2
> +     }
> +     -re ".*exited normally.*" {
> +         return 3
> +     }
> +     -re ".*The program is not being run.*" {
> +         return 4
> +     }
> +    }
> +    return 0
> +}
> +
> +set testfile "mips-64-r6"
> +set srcfile ${testfile}.c
> +set binfile ${objdir}/${subdir}/${testfile}

You should be able to use "standard_testfile" at the top to set those.

> +
> +verbose -log "\[DEBUG\] testfile=${testfile}\n"
> +verbose -log "\[DEBUG\] srcfile=${srcfile}\n"
> +verbose -log "\[DEBUG\] binfile=${binfile}\n"
> +verbose -log "\[DEBUG\] srcdir/subdir/srcfile=${srcdir}/${subdir}/${srcfile}\n\n"

That shouldn't be necessary, they are all standard variables used in all
tests.

> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug nowarnings}] != "" } {

Why the nowarnings?

> +     fail "compilation"

Use:

  untested "failed to compile"

> +     return
> +}
> +
> +pass "compilation"

Remove this "pass".

> +
> +clean_restart $binfile
> +# Native needs run.
> +runto_main

Use:

    if { ![runto_main } {
        return
    }

> +
> +set tests ""
> +foreach n [list "r6_branch" "r6_forbidden" "r6_64" "r6" "r6_fpu" "r6_llsc_dp" "r6_llsc_wp"] {
> +    lappend tests "test_$n"
> +}
> +
> +# put breakpoint on each test-function
> +foreach func $tests {
> +    gdb_test "break $func" "Breakpoint.*at.*" "set breakpoint on $func"
> +}

I don't really understand why you have a loop to fill the `tests` list,
then another loop.  Can't you just have:

set tests {r6_branch r6_forbidden ...}

foreach test $tests {
    gdb_test "break test_$test" ...
}

?

> +
> +
> +set rt [single_step]
> +if { $rt == 0 } {
> +    fail "single_step returned $rt"
> +}
> +
> +set start [timestamp]
> +global timeout
> +while { $rt != 0 && [timestamp] - $start < 3*$timeout } {
> +
> +     if { $rt == 3 } {
> +     pass "all tests are fine"
> +     return
> +    } elseif { $rt == 4 } {
> +     fail "Program exited abnormally"
> +     return
> +    }
> +#    elseif { $rt == 1 || $rt == 2 } { # 1->got gdb_prompt ; 2->hit breakpoint
> +#    verbose -log "\[DEBUG_\] 'single_step' returned rt=$rt ; timeout = $timeout"
> +#    }
> +
> +    set rt [single_step]
> +}
> +
> +if {$rt == 0 } {
> +    fail "stepi"
> +}

I don't really understand the test mechanic.  Can you describe what you
are trying to achieve?

Simon