From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id E13D43858C52 for ; Fri, 5 Apr 2024 15:12:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E13D43858C52 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org E13D43858C52 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712329954; cv=none; b=NkEMJ4+2dELDvyN0nVBaNdKB0wjJsyjm6kn2/lAWdxaDHUPF3zpQ18WCErgWaKWgSGgDrbMQb3bL/Qgbnf9vWZE8uCJnfvegPq3yNKuVgPRoeO8ZB9skk0ef+PNFtuqLVJJ6ZYZ3i0mGL8Lcv5SRWBjUGAR2M6K9hvbx7gr23fM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712329954; c=relaxed/simple; bh=bss7m4dkRuEVq77PLuNrpig4UWkxIY7BG4Tm0y/SZ9o=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=Fp11X6vqjf1K90ZBSw0n1X9lD63nwgaDLvN+QSFuBs9KDiFE45+6tzZMGPqy+wIi2dr7+SRMdnsUfNbc9h/YKYC67x0XYRQEA56uddT516dySRPU6bOy6bofPr0K9pH77WQ8xPiLVkfFYq/2ovIuvKIlVobUvwa3WE7O+ThE4Sg= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1712329948; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=mPrDq49qr/E1Zg0o+Fe57Kjt2LCblGI776znQeSlzvw=; b=VkOKC/u/poG/rda5yivp68BrgzWt1jl0J2cgJeN/Strhx49D8q68gsetdbvqmB2fh4A7Xe Dk/B5N/u5Czn6kIaKXoLoehW41aPR3g8Xkn9NjccdAwNh1ICTg/bEgm3Et6yPw5O+r0ZpP E1m+HGricFVOyLkjJ0T3gy1RFHcWWnU= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-401-hR0LWTK7N4SQWEfZ8X_WDA-1; Fri, 05 Apr 2024 11:12:26 -0400 X-MC-Unique: hR0LWTK7N4SQWEfZ8X_WDA-1 Received: by mail-ej1-f71.google.com with SMTP id a640c23a62f3a-a466c7b0587so140019366b.0 for ; Fri, 05 Apr 2024 08:12:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712329942; x=1712934742; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=mPrDq49qr/E1Zg0o+Fe57Kjt2LCblGI776znQeSlzvw=; b=Zqmh6JVj0Mj2JJBFejS0Okt9Dw7XruAe/UwCUFMPoTm//I3ME1+WnB6hOvpNBHSVGy fho4P+EFlbdjj+S3D1Er68+E6sGd1gWbE+RmB0w3Xs4+hvk4gpjgBSxFVrEHz27A/Mee bXPPjGMnCBNa+rGpuHqBE3o2q3nMZenIP7fpe/Zz9CTcw3TIzGfkOh6WDj9G1+dKYxSL 7z/EnykCuV63POLfEb6rCjOPimBUVKIIkJqQqvPbCeOorYYcrdWkq9gnxHhIS/MlBG6I dx822vwmRZeTw9WU/Q3Rw0AxUH4KiaNqHVi23GDY7N4CbL/WqObQZm+Ppe/nQ1Yw7eHz 1VDg== X-Forwarded-Encrypted: i=1; AJvYcCXGqWzxjmX3osCgFTngTcw0vuvn6yQVgQ3oB8aqRJDAfWzQv2MexqFJetnApzfqlSWE3pklj+ZWS+/GyMx3qm9bxLkvEzSn4fDR7A== X-Gm-Message-State: AOJu0Yy9EepP6psWa1v0TgoKB8uIDeZosYR6PzGAsL4tbxlzhKi28zGj kfiZCTf4pE1AK+U5T7Qkfj8yXijjuf1bb0U84n1WH4h5X4SZGGg2kmJawMpMdxgrm+i3d2wnKce Q/mP0ffInOctfVtfMro45xBxd6DdPv1LrXGVNZJBdqAvKchuszxFemlFAeEo= X-Received: by 2002:a17:906:f8d0:b0:a4e:38c0:8730 with SMTP id lh16-20020a170906f8d000b00a4e38c08730mr801297ejb.77.1712329941712; Fri, 05 Apr 2024 08:12:21 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHCd1FcfCktV9FwAHIwUm2Dcd0AbCNIAU2hFZ4KxP04WIUFeqvrz+FstjY9YPGlnFaQVNCf3w== X-Received: by 2002:a17:906:f8d0:b0:a4e:38c0:8730 with SMTP id lh16-20020a170906f8d000b00a4e38c08730mr801271ejb.77.1712329940610; Fri, 05 Apr 2024 08:12:20 -0700 (PDT) Received: from localhost (185.223.159.143.dyn.plus.net. [143.159.223.185]) by smtp.gmail.com with ESMTPSA id gx5-20020a1709068a4500b00a4e579ce949sm938396ejc.51.2024.04.05.08.12.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Apr 2024 08:12:20 -0700 (PDT) From: Andrew Burgess To: Milos Kalicanin , "gdb-patches@sourceware.org" Cc: "Maciej W . Rozycki" , Chao-ying Fu , Simon Marchi , Djordje Todorovic , Dragan Mladjenovic Subject: Re: [PATCH^4] gdb: mips: Add MIPSR6 support In-Reply-To: References: Date: Fri, 05 Apr 2024 16:12:19 +0100 Message-ID: <87frvz3kto.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Milos Kalicanin writes: > 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. OK, so I've not done any MIPS work in ~20 years, so I'm not going to review the actual details of the MIPS architecture that you've implemented here. I see you've CC'd the MIPS maintainer, they can comment if they care. All the changes are contained in the mips tdep file, so if you break it, then it's not going to hurt anyone else. I do have some style comments which I think should be addressed, all notes are inline below. > > Changes from v3: Apply code formatting as Simon Marchi suggested. > > [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-02-14 Andrew Bennett > Matthew Fortune > Faraz Shahbazker > > 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 | 538 +++++++++++++-- > gdb/testsuite/gdb.arch/mips-64-r6.c | 913 ++++++++++++++++++++++++++ > gdb/testsuite/gdb.arch/mips-64-r6.exp | 88 +++ > 3 files changed, 1492 insertions(+), 47 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..609932100cf 100644 > --- a/gdb/mips-tdep.c > +++ b/gdb/mips-tdep.c > @@ -60,7 +60,7 @@ > > static struct type *mips_register_type (struct gdbarch *gdbarch, int regnum); > > -static int mips32_instruction_has_delay_slot (struct gdbarch *gdbarch, > +static bool mips32_instruction_has_delay_slot (struct gdbarch *gdbarch, > ULONGEST inst); > static int micromips_instruction_has_delay_slot (ULONGEST insn, int mustbe32); > static int mips16_instruction_has_delay_slot (unsigned short inst, > @@ -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) > @@ -124,6 +127,10 @@ enum mips_breakpoint_kind > MIPS_BP_KIND_MICROMIPS32 = 5, > }; > > +/* Enum describing type of compact branch or jump. */ Two spaces at the end of the sentence before the '*/'. > +enum compact_branch_type > +{ COMP_BRANCH_NONE = 0, COMP_BRANCH_UNCOND = 1, COMP_BRANCH_COND = 2 }; > + > /* For backwards compatibility we default to MIPS16. This flag is > overridden as soon as unambiguous ELF file flags tell us the > compressed ISA encoding used. */ > @@ -1500,6 +1507,16 @@ mips_fetch_instruction (struct gdbarch *gdbarch, > return extract_unsigned_integer (buf, instlen, byte_order); > } > > +/* Return true if the gdbarch is based on MIPS Release 6. */ > +static bool > +is_mipsr6_isa (struct gdbarch *gdbarch) > +{ > + const struct bfd_arch_info *info = gdbarch_bfd_arch_info (gdbarch); > + > + return (info->mach == bfd_mach_mipsisa32r6 > + || info->mach == bfd_mach_mipsisa64r6); > +} I notice that we already have the functions is_mips16_isa and is_micromips_isa, which return a result based on the value of mips_gdbarch_tdep::mips_isa. I suspect I can guess the answer, but I think it would be good to place this function next to the other *_isa () functions, and I think the comment on the function should explain why this function is different to the other functions. > + > /* These are the fields of 32 bit mips instructions. */ > #define mips32_op(x) (x >> 26) > #define itype_op(x) (x >> 26) > @@ -1542,6 +1559,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) > @@ -1636,6 +1666,71 @@ is_octeon_bbit_op (int op, struct gdbarch *gdbarch) > return 0; > } > > +static bool > +is_add32bit_overflow (int32_t a, int32_t b) > +{ > + int32_t r = (uint32_t) a + (uint32_t) b; > + return (a < 0 && b < 0 && r >= 0) || (a >= 0 && b >= 0 && r < 0); > +} > + > +static bool > +is_add64bit_overflow (int64_t a, int64_t b) > +{ > + if (a != (int32_t) a) > + return true; > + if (b != (int32_t) b) > + return true; > + return is_add32bit_overflow ((int32_t) a, (int32_t) b); > +} Every function should have a header comment explaining the purpose. > + > +/* Calculate address of next instruction after BLEZ. */ And ideally the comment should explain what the function arguments actually mean. Some it's OK to skip, e.g. gdbarch and regcache. But at a minimum this function should explain INST, PC, and INVERT. > + > +static CORE_ADDR > +mips32_blez_pc (struct gdbarch *gdbarch, struct regcache *regcache, > + 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); > + bool taken = false; > + int delay_slot_size = 4; > + > + /* BLEZ, BLEZL, BGTZ, BGTZL */ This comment doesn't really help me. Are you detecting these instructions? Handling these functions after already detecting them? > + 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; */ Clearly my lack of MIPS knowledge doesn't help here. I assume the commented out code is related to the comment in some way, but it's not clear to me how exactly. I'd prefer the comment to say something like: "If we had access to XXX then we'd know YYY and so we'd set delay_slot_size to 0. We don't have access to XXX because ZZZ, so we leave delay_slot_size as is. This might case ... problems / This is fine because of reason ...". > + } > + > + 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. */ > @@ -2451,6 +2690,72 @@ micromips_instruction_is_compact_branch (unsigned short insn) > } > } > > + > + > +/* Return non-zero if the MIPS instruction INSN is a compact branch > + or jump. */ > +static enum compact_branch_type Double space at end of sentence. Also you're a little inconsistent about whether you leave a empty line after the comment and before the function. I believe the GDB style is to leave a blank line. > +mips32_instruction_is_compact_branch (struct gdbarch *gdbarch, ULONGEST insn) > +{ > + switch (itype_op (insn)) > + { > + /* BC */ > + case 50: > + /* BALC */ > + case 58: > + if (is_mipsr6_isa (gdbarch)) > + return COMP_BRANCH_UNCOND; > + break; > + /* BOVC, BEQZALC, BEQC */ > + case 8: > + /* BNVC, BNEZALC, BNEC */ > + case 24: > + if (is_mipsr6_isa (gdbarch)) > + return COMP_BRANCH_COND; > + break; > + /* BEQZC, JIC */ > + case 54: > + /* BNEZC, JIALC */ > + case 62: > + if (is_mipsr6_isa (gdbarch)) > + /* JIC, JIALC are unconditional */ > + return (itype_rs (insn) == 0) ? COMP_BRANCH_UNCOND : COMP_BRANCH_COND; > + 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 COMP_BRANCH_COND; > + break; > + /* BPOSGE32C */ > + case 1: > + if (is_mipsr6_isa (gdbarch) > + && itype_rt (insn) == 0x1d && itype_rs (insn) == 0) > + return COMP_BRANCH_COND; > + } > + return COMP_BRANCH_NONE; > +} > + > +/* Return true if a standard MIPS instruction at ADDR has a branch > + forbidden slot (i.e. it is a conditional compact branch instruction). */ > + > +static bool > +mips32_insn_at_pc_has_forbidden_slot (struct gdbarch *gdbarch, CORE_ADDR addr) > +{ > + int status; > + ULONGEST insn = mips_fetch_instruction (gdbarch, ISA_MIPS, addr, &status); > + if (status) > + return false; > + > + return mips32_instruction_is_compact_branch (gdbarch, insn) == COMP_BRANCH_COND; > +} > + > struct mips_frame_cache > { > CORE_ADDR base; > @@ -3938,6 +4246,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 bool > +is_ll_insn (struct gdbarch *gdbarch, ULONGEST insn) Missing header comment. > +{ > + if (itype_op (insn) == LL_OPCODE > + || itype_op (insn) == LLD_OPCODE) > + return true; > + > + if (rtype_op (insn) == LLSC_R6_OPCODE > + && rtype_funct (insn) == LLE_FUNCT > + && (insn & 0x40) == 0) > + return true; > + > + /* 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 true; > + > + return false; > +} > + > +static bool > +is_sc_insn (struct gdbarch *gdbarch, ULONGEST insn) Missing header comment. > +{ > + if (itype_op (insn) == SC_OPCODE > + || itype_op (insn) == SCD_OPCODE) > + return true; > + > + if (rtype_op (insn) == LLSC_R6_OPCODE > + && rtype_funct (insn) == SCE_FUNCT > + && (insn & 0x40) == 0) > + return true; > + > + /* Handle SC and SCP varieties. */ > + if (is_mipsr6_isa (gdbarch) > + && rtype_op (insn) == LLSC_R6_OPCODE > + && (rtype_funct (insn) == SC_R6_FUNCT > + || rtype_funct (insn) == SCD_R6_FUNCT > + || rtype_funct (insn) == SCE_FUNCT)) > + return true; > + > + return false; > +} > > static std::vector > mips_deal_with_atomic_sequence (struct gdbarch *gdbarch, CORE_ADDR pc) > @@ -3983,28 +4345,72 @@ mips_deal_with_atomic_sequence (struct gdbarch *gdbarch, CORE_ADDR pc) > return {}; /* fallback to the standard single-step code. */ > case 4: /* BEQ */ > case 5: /* BNE */ > - case 6: /* BLEZ */ > - case 7: /* BGTZ */ > case 20: /* BEQL */ > case 21: /* BNEL */ > - case 22: /* BLEZL */ > - case 23: /* BGTTL */ > + case 22: /* BLEZL (BLEZC, BGEZC, BGEC) */ > + case 23: /* BGTZL (BGTZC, BLTZC, BLTC) */ > + is_branch = 1; > + break; > + case 6: /* BLEZ (BLEZALC, BGEZALC, BGEUC) */ > + case 7: /* BGTZ (BGTZALC, BLTZALC, BLTUC) */ > + if (is_mipsr6) > + { > + /* BLEZALC, BGTZALC */ > + if (itype_rs (insn) == 0 && itype_rt (insn) != 0) > + return {}; /* fallback to the standard single-step code. */ > + /* BGEZALC, BLTZALC */ > + else if (itype_rs (insn) == itype_rt (insn) > + && itype_rt (insn) != 0) > + return {}; /* fallback to the standard single-step code. */ > + } > is_branch = 1; > break; > + case 8: /* BOVC, BEQZALC, BEQC */ > + case 24: /* BNVC, BNEZALC, BNEC */ > + if (is_mipsr6) > + is_branch = 1; > + break; > + case 50: /* BC */ > + case 58: /* BALC */ > + if (is_mipsr6) > + return {}; /* fallback to the standard single-step code. */ > + break; > + case 54: /* BEQZC, JIC */ > + case 62: /* BNEZC, JIALC */ > + if (is_mipsr6) > + { > + if (itype_rs (insn) == 0) /* JIC, JIALC */ > + return {}; /* fallback to the standard single-step code. */ > + else > + is_branch = 2; /* Marker for branches with a 21-bit offset. */ > + } > + break; > case 17: /* COP1 */ > - is_branch = ((itype_rs (insn) == 9 || itype_rs (insn) == 10) > - && (itype_rt (insn) & 0x2) == 0); > - if (is_branch) /* BC1ANY2F, BC1ANY2T, BC1ANY4F, BC1ANY4T */ > + is_branch = ((!is_mipsr6 > + /* BC1ANY2F, BC1ANY2T, BC1ANY4F, BC1ANY4T */ > + && (itype_rs (insn) == 9 || itype_rs (insn) == 10) > + && (itype_rt (insn) & 0x2) == 0) > + /* BZ.df: 010001 110xx */ > + || (itype_rs (insn) & 0x18) == 0x18); > + if (is_branch) > break; > [[fallthrough]]; > case 18: /* COP2 */ > case 19: /* COP3 */ > - is_branch = (itype_rs (insn) == 8); /* BCzF, BCzFL, BCzT, BCzTL */ > + /* BCzF, BCzFL, BCzT, BCzTL, BC*EQZ, BC*NEZ */ > + is_branch = ((itype_rs (insn) == 8) > + || (is_mipsr6 > + && (itype_rs (insn) == 9 > + || itype_rs (insn) == 13))); > break; > } > if (is_branch) Could you update this condition to: if (is_branch != 0) GDB is (slowly) moving away from using int as a bool replacement. And we try to avoid implicit conversion to bool in conditions (e.g. explicit nullptr comparisons). Especially now that you've introduced '2' as another value of 'is_branch' I think the condition should be explicit about what you're checking for. > { > - branch_bp = loc + mips32_relative_offset (insn) + 4; > + /* Is this a special PC21_S2 branch? */ > + if (is_branch == 2) > + branch_bp = loc + mips32_relative_offset21 (insn) + 4; > + else > + branch_bp = loc + mips32_relative_offset (insn) + 4; > if (last_breakpoint >= 1) > return {}; /* More than one branch found, fallback to the > standard single-step code. */ > @@ -4012,12 +4418,12 @@ mips_deal_with_atomic_sequence (struct gdbarch *gdbarch, CORE_ADDR pc) > last_breakpoint++; > } > > - if (itype_op (insn) == SC_OPCODE || itype_op (insn) == SCD_OPCODE) > + if (is_sc_insn (gdbarch, insn)) > break; > } > > /* Assume that the atomic sequence ends with a sc/scd instruction. */ > - if (itype_op (insn) != SC_OPCODE && itype_op (insn) != SCD_OPCODE) > + if (!is_sc_insn (gdbarch, insn)) > return {}; > > loc += MIPS_INSN32_SIZE; > @@ -4231,7 +4637,7 @@ mips_software_single_step (struct regcache *regcache) > /* Test whether the PC points to the return instruction at the > end of a function. */ > > -static int > +static bool > mips_about_to_return (struct gdbarch *gdbarch, CORE_ADDR pc) > { > ULONGEST insn; > @@ -4242,8 +4648,14 @@ mips_about_to_return (struct gdbarch *gdbarch, CORE_ADDR pc) > gdb_assert (mips_pc_is_mips (pc)); > > insn = mips_fetch_instruction (gdbarch, ISA_MIPS, pc, NULL); > - hint = 0x7c0; > - return (insn & ~hint) == 0x3e00008; /* jr(.hb) $ra */ > + /* Mask the hint and the jalr/jr bit. */ > + hint = 0x7c1; > + > + if (is_mipsr6_isa (gdbarch) && insn == 0xd81f0000) /* jrc $31 */ > + return true; > + > + /* jr(.hb) $ra and "jalr(.hb) $ra" */ > + return ((insn & ~hint) == 0x3e00008); > } > > > @@ -6760,7 +7172,9 @@ mips32_stack_frame_destroyed_p (struct gdbarch *gdbarch, CORE_ADDR pc) > > if (high_word != 0x27bd /* addiu $sp,$sp,offset */ > && high_word != 0x67bd /* daddiu $sp,$sp,offset */ > - && inst != 0x03e00008 /* jr $ra */ > + && (inst & ~0x1) != 0x03e00008 /* jr $31 or jalr $0, $31 */ > + && (!is_mipsr6_isa (gdbarch) > + || inst != 0xd81f0000) /* jrc $31 */ > && inst != 0x00000000) /* nop */ > return 0; > } > @@ -7129,32 +7543,41 @@ mips_sw_breakpoint_from_kind (struct gdbarch *gdbarch, int kind, int *size) > }; > } > > -/* Return non-zero if the standard MIPS instruction INST has a branch > +/* Return true if the standard MIPS instruction INST has a branch > delay slot (i.e. it is a jump or branch instruction). This function > is based on mips32_next_pc. */ > > -static int > +static bool > mips32_instruction_has_delay_slot (struct gdbarch *gdbarch, ULONGEST inst) > { > int op; > int rs; > int rt; > + int is_mipsr6 = is_mipsr6_isa (gdbarch); > > op = itype_op (inst); > if ((inst & 0xe0000000) != 0) > { > rs = itype_rs (inst); > rt = itype_rt (inst); > - return (is_octeon_bbit_op (op, gdbarch) > - || op >> 2 == 5 /* BEQL, BNEL, BLEZL, BGTZL: bits 0101xx */ > - || op == 29 /* JALX: bits 011101 */ > + return (is_octeon_bbit_op (op, gdbarch) > + || (op >> 1 == 10) /* BEQL, BNEL: bits 01010x */ > + || (op >> 1 == 11 && rt == 0) /* BLEZL, BGTZL: bits 01011x */ > + || (!is_mipsr6 && op == 29) /* JALX: bits 011101 */ > || (op == 17 > && (rs == 8 > /* BC1F, BC1FL, BC1T, BC1TL: 010001 01000 */ > - || (rs == 9 && (rt & 0x2) == 0) > + || (!is_mipsr6 && rs == 9 && (rt & 0x2) == 0) > /* BC1ANY2F, BC1ANY2T: bits 010001 01001 */ > - || (rs == 10 && (rt & 0x2) == 0)))); > + || (!is_mipsr6 && rs == 10 && (rt & 0x2) == 0))) > /* BC1ANY4F, BC1ANY4T: bits 010001 01010 */ > + || (is_mipsr6 > + && ((op == 17 > + && (rs == 9 /* BC1EQZ: 010001 01001 */ > + || rs == 13)) /* BC1NEZ: 010001 01101 */ > + || (op == 18 > + && (rs == 9 /* BC2EQZ: 010010 01001 */ > + || rs == 13))))); /* BC2NEZ: 010010 01101 */ > } > else > switch (op & 0x07) /* extract bits 28,27,26 */ > @@ -7173,8 +7596,12 @@ mips32_instruction_has_delay_slot (struct gdbarch *gdbarch, ULONGEST inst) > || ((rt & 0x1e) == 0x1c && rs == 0)); > /* BPOSGE32, BPOSGE64: bits 1110x */ > break; /* end REGIMM */ > - default: /* J, JAL, BEQ, BNE, BLEZ, BGTZ */ > - return 1; > + case 6: /* BLEZ */ > + case 7: /* BGTZ */ > + return (itype_rt (inst) == 0); > + break; > + default: /* J, JAL, BEQ, BNE */ > + return true; > break; > } > } > @@ -7385,7 +7812,18 @@ mips_adjust_breakpoint_address (struct gdbarch *gdbarch, CORE_ADDR bpaddr) > > So, we'll use the second solution. To do this we need to know if > the instruction we're trying to set the breakpoint on is in the > - branch delay slot. */ > + branch delay slot. > + > + A similar problem occurs for breakpoints on forbidden slots where > + the trap will be reported for the branch with the BD bit set. > + In this case it would be ideal to recover using solution 1 from > + above as there is no problem with the branch being skipped > + (since the forbidden slot only exists on not-taken branches). > + However, the BD bit is not available in all scenarios currently > + so instead we move the breakpoint on to the next instruction. > + This means that it is not possible to stop on an instruction > + that can be in a forbidden slot even if that instruction is > + jumped to directly. */ > > boundary = mips_segment_boundary (bpaddr); > > @@ -7407,6 +7845,12 @@ mips_adjust_breakpoint_address (struct gdbarch *gdbarch, CORE_ADDR bpaddr) > prev_addr = bpaddr - 4; > if (mips32_insn_at_pc_has_delay_slot (gdbarch, prev_addr)) > bpaddr = prev_addr; > + /* If the previous instruction has a forbidden slot, we have to > + move the breakpoint to the following instruction to prevent > + breakpoints in forbidden slots being reported as unknown > + traps. */ > + else if (mips32_insn_at_pc_has_forbidden_slot (gdbarch, prev_addr)) > + bpaddr += 4; > } > else > { > diff --git a/gdb/testsuite/gdb.arch/mips-64-r6.c b/gdb/testsuite/gdb.arch/mips-64-r6.c > new file mode 100644 > index 00000000000..cd540d1e70c > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/mips-64-r6.c > @@ -0,0 +1,913 @@ > +/* > + Copyright 2006-2024 Free Software Foundation, Inc. So, for a long time I thought that the date range here was supposed to be the years that the code was offered to upstream GDB. But I think I saw something recently that these dates should actually be the range for when this code was "released" whatever that means, even if that release wasn't for upstream GDB. But what I did notice was that this .c file has a different date range than the corresponding .exp file, which seems weird (but not impossible I guess). I think my point here is: can you check these date ranges. If you've released these files previously then I guess retain the original release year, but if this is the first time the file is being released, then you likely just need 2023 - 2024 here ... maybe? > + > + This file is part of GDB. > + > + 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 . > +*/ > + > +#define xstr(s) str(s) > +#define str(s) #s > + > +/* ============ macros from sim/testutils/mips/utils-r6.inc ============= */ > + > +/* 58 is local label to exit with errcode != 0 */ > +#define fp_assert(a, b) "beq " xstr (a) ", " xstr (b) ", 1f \n\t" \ > + "nop \n\t" \ > + "b 58f \n\t" \ > + "nop \n\t" \ > + "1: \n\t" > + > +/* Clobbers: $4,$6,$7 */ > +#define r6ck_1r(inst, a, ret) \ > + "li $4, " xstr (a) " \n\t" \ > + "li $6, " xstr (ret) " \n\t" \ > + xstr (inst) " $7, $4 \n\t" \ > + fp_assert ($6, $7) > + > +/* Clobbers: $4,$6,$7 */ > +#define r6ck_1dr(inst, a, ret) \ > + "ld $4, " xstr (a) " \n\t" \ > + "ld $6, " xstr (ret) " \n\t" \ > + xstr (inst) " $7, $4 \n\t" \ > + fp_assert ($6, $7) > + > +/* Clobbers: $4,$5,$6,$7 */ > +#define r6ck_2r(inst, a, b, ret) \ > + "li $4, " xstr (a) " \n\t" \ > + "li $5, " xstr (b) " \n\t" \ > + "li $6, " xstr (ret) " \n\t" \ > + xstr (inst) " $7, $4, $5 \n\t" \ > + fp_assert ($6, $7) > + > +/* Clobbers: $4,$5,$6,$7 */ > +#define r6ck_2dr(inst, a, b, ret) \ > + "ld $4, " xstr (a) " \n\t" \ > + "ld $5, " xstr (b) " \n\t" \ > + "ld $6, " xstr (ret) " \n\t" \ > + xstr (inst) " $7, $4, $5 \n\t" \ > + fp_assert ($6, $7) > + > +/* Clobbers: $4,$5,$6,$7 */ > +#define r6ck_2dr1i(inst, a, b, imm, ret) \ > + "ld $4, " xstr (a) " \n\t" \ > + "ld $5, " xstr (b) " \n\t" \ > + "ld $6, " xstr (ret) " \n\t" \ > + xstr (inst) " $7, $4, $5, " xstr (imm) " \n\t" \ > + fp_assert ($6, $7) > + > +/* Clobbers: $4,$6,$7 */ > +#define r6ck_1r1i(inst, a, imm, ret) \ > + "li $4, " xstr (a) " \n\t" \ > + "li $6, " xstr (ret) " \n\t" \ > + xstr (inst) " $7, $4, " xstr (imm) " \n\t" \ > + fp_assert ($6, $7) > + > +/* Clobbers: $4,$6,$7 */ > +#define r6ck_1dr1i(inst, a, imm, ret) \ > + "ld $4, " xstr (a) " \n\t" \ > + "ld $6, " xstr (ret) " \n\t" \ > + xstr (inst) " $7, $4, " xstr (imm) " \n\t" \ > + fp_assert ($6, $7) > + > +/* Clobbers: $4,$6 */ > +#define r6ck_0dr1i(inst, a, imm, ret) \ > + "ld $4, " xstr (a) " \n\t" \ > + "ld $6, " xstr (ret) " \n\t" \ > + xstr (inst) " $4, $4, " xstr (imm) " \n\t" \ > + fp_assert ($6, $4) > + > +/* Clobbers: $4,$5,$6,$7 */ > +#define r6ck_2r1i(inst, a, b, imm, ret) \ > + "li $4, " xstr (a) " \n\t" \ > + "li $5, " xstr (b) " \n\t" \ > + "li $6, " xstr (ret) " \n\t" \ > + xstr (inst) " $7, $4, $5, " xstr (imm) " \n\t" \ > + fp_assert ($6, $7) > + > +/* Clobbers: $4,$5,$6,$7,$8,$f2,$f4,$f6 */ > +#define r6ck_3s(inst, a, b, c, ret) \ > + "li $4, " xstr (a) " \n\t" \ > + "li $5, " xstr (b) " \n\t" \ > + "li $6, " xstr (c) " \n\t" \ > + "li $7, " xstr (ret) " \n\t" \ > + "mtc1 $4, $f2 \n\t" \ > + "mtc1 $5, $f4 \n\t" \ > + "mtc1 $6, $f6 \n\t" \ > + xstr (inst) " $f2, $f4, $f6 \n\t" \ > + "mfc1 $8, $f2 \n\t" \ > + fp_assert ($7, $8) > + > +/* Clobbers: $4,$5,$6,$7,$f2,$f4 */ > +#define r6ck_2s(inst, a, b, ret) \ > + "li $4, " xstr (a) " \n\t" \ > + "li $5, " xstr (b) " \n\t" \ > + "li $6, " xstr (ret) " \n\t" \ > + "mtc1 $4, $f2 \n\t" \ > + "mtc1 $5, $f4 \n\t" \ > + xstr (inst) " $f2, $f4 \n\t" \ > + "mfc1 $7, $f2 \n\t" \ > + fp_assert ($6, $7) > + > +/* Clobbers: $4,$5,$6,$7,$8,$9,$10,$f2,$f4 */ > +#define r6ck_2d(inst, a, b, ret) \ > + ".data \n\t" \ > + "1: .dword " xstr (a) " \n\t" \ > + "2: .dword " xstr (b) " \n\t" \ > + "3: .dword " xstr (ret) " \n\t" \ > + ".text \n\t" \ > + "dla $4, 1b \n\t" \ > + "dla $5, 2b \n\t" \ > + "dla $6, 3b \n\t" \ > + "ldc1 $f2, 0($4) \n\t" \ > + "ldc1 $f4, 0($5) \n\t" \ > + "lw $7, 0($6) \n\t" \ > + "lw $8, 4($6) \n\t" \ > + xstr (inst) " $f2, $f4 \n\t" \ > + "mfhc1 $9, $f2 \n\t" \ > + "mfc1 $10, $f2 \n\t" \ > + fp_assert ($7, $9) \ > + fp_assert ($8, $10) > + > +/* Clobbers: $2,$4,$5,$6,$7,$8,$9,$10,$f2,$f4,$f6 */ > +#define r6ck_3d(inst, a, b, c, ret) \ > + ".data \n\t" \ > + "1: .dword " xstr (a) " \n\t" \ > + "2: .dword " xstr (b) " \n\t" \ > + "3: .dword " xstr (c) " \n\t" \ > + "4: .dword " xstr (ret) " \n\t" \ > + ".text \n\t" \ > + "dla $4, 1b \n\t" \ > + "dla $5, 2b \n\t" \ > + "dla $6, 3b \n\t" \ > + "dla $2, 4b \n\t" \ > + "ldc1 $f2, 0($4) \n\t" \ > + "ldc1 $f4, 0($5) \n\t" \ > + "ldc1 $f6, 0($6) \n\t" \ > + "lw $7, 0($2) \n\t" \ > + "lw $8, 4($2) \n\t" \ > + xstr (inst) " $f2, $f4, $f6 \n\t" \ > + "mfhc1 $9, $f2 \n\t" \ > + "mfc1 $10, $f2 \n\t" \ > + fp_assert ($7, $9) \ > + fp_assert ($8, $10) > + > + > +/* ============ macros from sim/testutils/mips/testutils.inc ============= */ > + > +/* Put value 'val' into register 'reg' > + * Clobbers: None */ > +#define load32(reg, val) \ > + "li " xstr (reg) ", " xstr (val) " \n\t" > + > +/* Check whether two registers contain the same value > + * Clobbers: None */ > +#define checkreg(reg, expreg) \ > + ".set push \n\t" \ > + ".set noat \n\t" \ > + ".set noreorder \n\t" \ > + "beq " xstr (expreg) ", " xstr (reg) ", 901f \n\t" \ > + "nop \n\t" \ > + "b 58f \n\t" \ > + "nop \n\t" \ > + "901: \n\t" \ > + ".set pop \n\t" > + > +/* Check if register 'reg' contains value 'val'. > + * Clobbers: $1 */ > +#define check32(reg, val) \ > + ".set push \n\t" \ > + ".set noat \n\t" \ > + load32 ($1, val) \ > + checkreg (reg, $1) \ > + ".set pop \n\t" > + > +/* Checkpair based on endianess > + * Clobbers: $1 */ > +#define checkpair_xendian(lo, hi, base, ec, w) \ > + ".set noat \n\t" \ > + "lw $1, " xstr (ec) " \n\t" \ > + "andi $1, $1, 0x1 \n\t" \ > + "beqz $1, 2f \n\t" \ > + ".set at \n\t" \ > + "1: \n\t" \ > + checkpair_be_##w (lo, hi, base) \ > + "b 3f \n\t" \ > + "nop \n\t" \ > + "2: \n\t" \ > + checkpair_le_##w (lo, hi, base) \ > + "3: \n\t" > + > + > +/* Check hi-lo register pair against data stored at base+o1 and base+o2 > + * Clobbers: $1 - $5 */ > +#define checkpair(lo, hi, base, w, o1, o2) \ > + "move $2, " xstr (lo) " \n\t" \ > + "move $3, " xstr (hi) " \n\t" \ > + ".set noat \n\t" \ > + "dla $1, " xstr (base) " \n\t" \ > + "l" xstr (w) " $4, " xstr (o1) "($1) \n\t" \ > + "l" xstr (w) " $5, " xstr (o2) "($1) \n\t" \ > + ".set at \n\t" \ > + checkreg ($2, $4) \ > + checkreg ($3, $5) > + > +#define checkpair_le_d(lo, hi, base) \ > + checkpair (lo, hi, base, w, 0, 4) > + > +#define checkpair_be_d(lo, hi, base) \ > + checkpair (lo, hi, base, w, 4, 0) > + > + > +#define checkpair_le_q(lo, hi, base) \ > + checkpair (lo, hi, base, d, 0, 8) > + > +#define checkpair_be_q(lo, hi, base) \ > + checkpair (lo, hi, base, d, 8, 0) > + > +#define checkpair_qword(lo, hi, base, oe) \ > + checkpair_xendian (lo, hi, base, oe, q) > + > +#define checkpair_dword(lo, hi, base, oe) \ > + checkpair_xendian (lo, hi, base, oe, d) > + > +void > +abort (void); > + > +int > +test_r6_branch(void) > +{ > +/* Using volatile to prevent certain optimizations which could cause > + * instruction deletion. > + * 'err' identifies instruction which (eventually) caused error. > + * (err == 0) ==> all instructions executed successfully */ > + > + volatile int err = -1; > + volatile int a14 = 0xffffffff; > + volatile int a13 = 0x123; > + volatile int a12 = 0x45; > + volatile int a7 = 0x45; > + volatile int a8 = 0xfffffffe; > + volatile int a9 = 2147483647; > + volatile int a11 = 0; > + volatile int a10 = 0; > + > + asm ( > + ".set push \n\t" /* create new scope for asm configuration */ > + ".set noreorder \n\t" /* don't allow reordering of instructions */ > + "li %[err], 1 \n\t" > + "bovc %[a12], %[a13], Lfail \n\t" /* BOVC */ > + "nop \n\t" > + "bovc %[a9], %[a13], L2 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L2: \n\t" > + "li %[err], 2 \n\t" > + "bnvc %[a9], %[a13], Lfail \n\t" /* BNVC */ > + "nop \n\t" > + "bnvc %[a12], %[a13], L3 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L3: \n\t" > + "li %[err], 3 \n\t" > + "beqc %[a12], %[a13], Lfail \n\t" /* BEQC */ > + "nop \n\t" > + "beqc %[a12], %[a7], L4 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L4: \n\t" > + "li %[err], 4 \n\t" > + "bnec %[a12], %[a7], Lfail \n\t" /* BNEC */ > + "nop \n\t" > + "bnec %[a12], %[a13], L5 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L5: \n\t" > + "li %[err], 5 \n\t" > + "bltc %[a13], %[a12], Lfail \n\t" /* BLTC */ > + "nop \n\t" > + "bltc %[a12], %[a13], L6 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L6: \n\t" > + "L7: \n\t" > + "li %[err], 7 \n\t" > + "bgec %[a12], %[a13], Lfail \n\t" /* BGEC */ > + "nop \n\t" > + "bgec %[a13], %[a12], L8 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L8: \n\t" > + "L9: \n\t" > + "li %[err], 9 \n\t" > + "bltuc %[a14], %[a13], Lfail \n\t" /* BLTUC */ > + "nop \n\t" > + "bltuc %[a8], %[a14], L10 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L10: \n\t" > + "L11: \n\t" > + "li %[err], 11 \n\t" > + "bgeuc %[a13], %[a14], Lfail \n\t" /* BGEUC */ > + "nop \n\t" > + "bgeuc %[a14], %[a8], L12 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L12: \n\t" > + "L13: \n\t" > + "li %[err], 13 \n\t" > + "bltzc %[a13], Lfail \n\t" /* BLTZC */ > + "nop \n\t" > + "bltzc %[a11], Lfail \n\t" > + "nop \n\t" > + "bltzc %[a14], L14 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L14: \n\t" > + "li %[err], 14 \n\t" > + "blezc %[a13], Lfail \n\t" /* BLEZC */ > + "nop \n\t" > + "blezc %[a11], L145 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L145: \n\t" > + "blezc %[a14], L15 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L15: \n\t" > + "li %[err], 15 \n\t" > + "bgezc %[a8], Lfail \n\t" /* BGEZC */ > + "nop \n\t" > + "bgezc %[a11], L155 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L155: \n\t" > + "bgezc %[a13], L16 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L16: \n\t" > + "li %[err], 16 \n\t" > + "bgtzc %[a8], Lfail \n\t" /* BGTZC */ > + "nop \n\t" > + "bgtzc %[a11], Lfail \n\t" > + "nop \n\t" > + "bgtzc %[a13], L17 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "li %[a10], 0 \n\t" > + "L17: \n\t" > + "li %[err], 17 \n\t" > + "blezalc %[a12], Lfail \n\t" /* BLEZALC */ > + "nop \n\t" > + "blezalc %[a11], Lret \n\t" > + "li %[a10], 1 \n\t" > + "beqzc %[a10], L175 \n\t" /* BEQZC */ > + "nop \n\t" > + "li %[err], 8531 \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L175: \n\t" > + "li %[err], 23531 \n\t" > + "blezalc %[a14], Lret \n\t" > + "li %[a10], 1 \n\t" > + "beqzc %[a10], L18 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L18: \n\t" > + "li %[err], 18 \n\t" > + "bgezalc %[a14], Lfail \n\t" /* BGEZALC */ > + "nop \n\t" > + "bgezalc %[a11], Lret \n\t" > + "li %[a10], 1 \n\t" > + "beqzc %[a10], L185 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L185: \n\t" > + "bgezalc %[a12], Lret \n\t" > + "li %[a10], 1 \n\t" > + "beqzc %[a10], L19 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L19: \n\t" > + "li %[err], 19 \n\t" > + "bgtzalc %[a14], Lfail \n\t" /* BGTZALC */ > + "nop \n\t" > + "bgtzalc %[a11], Lfail \n\t" > + "nop \n\t" > + "bgtzalc %[a12], Lret \n\t" > + "li %[a10], 1 \n\t" > + "beqzc %[a10], L20 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L20: \n\t" > + "li %[err], 20 \n\t" > + "bltzalc %[a12], Lfail \n\t" /* BLTZALC */ > + "nop \n\t" > + "bltzalc %[a11], Lfail \n\t" > + "nop \n\t" > + "bltzalc %[a14], Lret \n\t" > + "li %[a10], 1 \n\t" > + "beqzc %[a10], L21 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L21: \n\t" > + "li %[err], 21 \n\t" > + "bc L22 \n\t" /* BC */ > + "b Lfail \n\t" > + "nop \n\t" > + "L22: \n\t" > + "li %[err], 22 \n\t" > + "balc Lret \n\t" /* BALC */ > + "li %[a10], 1 \n\t" > + "beqzc %[a10], L23 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L23: \n\t" > + "li %[err], 23 \n\t" > + "jal GetPC \n\t" /* JAL */ > + "nop \n\t" > + "jic $6, 4 \n\t" /* JIC */ > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L24: \n\t" > + "li %[err], 24 \n\t" > + "li %[a10], 1 \n\t" > + "jal GetPC \n\t" > + "nop \n\t" > + "jialc $6, 20 \n\t" /* JIALC */ > + "nop \n\t" > + "beqzc %[a10], L25 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "LJIALCRET: \n\t" > + "li %[a10], 0 \n\t" > + "jr $31 \n\t" /* JR */ > + "nop \n\t" > + "L25: \n\t" > + "li %[err], 25 \n\t" > + "jal GetPC \n\t" > + "nop \n\t" > + "move %[a11], $6 \n\t" > + "nal \n\t" > + "nop \n\t" > + "addiu %[a11], 12 \n\t" /* ADDIU */ > + "beqc %[a11], $31, L26 \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "L26: \n\t" > + "li %[err], 26 \n\t" > + "balc Lret \n\t" > + "li %[a10], 1 \n\t" > + "beqzc %[a10], Lend \n\t" > + "nop \n\t" > + "b Lfail \n\t" > + "nop \n\t" > + "Lret: \n\t" > + "li %[a10], 0 \n\t" > + "daddiu $31, 4 \n\t" /* DADDIU */ > + "jrc $31 \n\t" /* JRC */ > + "nop \n\t" > + "GetPC: \n\t" > + "move $6, $31 \n\t" > + "jr $31 \n\t" > + "Lend: \n\t" > + "li %[err], 0 \n\t" > + "Lfail: \n\t" > + ".set pop \n\t" /* Restore previous config */ > + : [err] "+r"(err), [a14] "+r"(a14), [a13] "+r"(a13), [a12] "+r"(a12), > + [a7] "+r"(a7), [a8] "+r"(a8), [a9] "+r"(a9), [a10] "+r"(a10), [a11] "+r"(a11) > + : /* inputs */ > + : "$31", "$6" /* clobbers */ > + ); > + > + return err; > +} > + > +int > +test_r6_forbidden(void) Test should follow GNU coding standard where possible, so space after the function name before the parameter list. This applies in a bunch of places in this file. > +{ > + volatile int err = -1; > + volatile int a4 = 0; > + volatile int a2 = 0; > + volatile int a1 = 0; > + > + asm ( > + ".set push \n\t" > + ".set noreorder \n\t" > +/* Test if FS is ignored when branch is taken */ > + "li %[err], 1 \n\t" > + "li %[a4], 0 \n\t" > + "beqzalc %[a4], L41 \n\t" > + "li %[err], -85 \n\t" > + "L42: \n\t" > + "b Lfail2 \n\t" > + "nop \n\t" > + "L41: \n\t" > + "blez %[err], Lfail2 \n\t" > +/* Test if FS is used when branch is not taken */ > + "li %[err], 2 \n\t" > + "li %[a4], 1 \n\t" > + "blezc %[a4], L43 \n\t" > + "addiu %[a4], %[a4], 1 \n\t" > + "li %[a2], 2 \n\t" > + "beq %[a4], %[a2], L44 \n\t" > + "nop \n\t" > + "L43: \n\t" > + "nop \n\t" > + "b Lfail2 \n\t" > + "nop \n\t" > + "L44: \n\t" > +/* Test if FS causes an error when it contains a branch */ > + "li %[err], 3 \n\t" > + "li %[a4], 3 \n\t" > + "beqzalc %[a4], Lfail2 \n\t" > +/* Note: bc L45 here causes SegFault: Illegal instruction which is OK. */ Double space at end of sentence. > 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..cddf9c19574 > --- /dev/null > +++ b/gdb/testsuite/gdb.arch/mips-64-r6.exp > @@ -0,0 +1,88 @@ > +# Copyright (C) 2012-2023 Free Software Foundation, Inc. Date range needs updating to 2024. Also this is the range that doesn't match the corresponding .c file ... which might be fine if that's actually true. > + > +# 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 . > + > +# Test mips release 6 patch. > + > +require {istarget "*mips*"} > + > +standard_testfile .c No need for the '.c' here -- it's the default value. > + > +set tests { test_r6_branch test_r6_forbidden test_r6_64 test_r6 > +test_r6_fpu test_r6_llsc_dp test_r6_llsc_wp } > + > +proc single_step {} { All procs should have a comment, but especially in this case with the set of return values being used. > + global gdb_prompt > + > + send_gdb "si\n" > + gdb_expect { I wonder if we could switch to gdb_test_multiple here, which catches a bunch of additional error outputs? > + -re "$gdb_prompt \$" { > + return 1 > + } > + -re ".*Breakpoint.*test_.*" { > + return 2 > + } > + -re ".*exited normally.*" { > + return 3 > + } In some embedded situations inferior's never really exit, but instead just enter a spin loop. As such, unless the test specifically requires a test for the inferior exiting, I always think it's better to place a breakpoint before the test exits and then stop the test once we hit that breakpoint. Maybe this concern doesn't apply for this MIPS variant? But it shouldn't be a difficult change. > + -re ".*The program is not being run.*" { > + return 4 > + } > + } > + return 0 > +} > + > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable { debug }] != "" } { > + fail "compilation" > + return > +} > + > +clean_restart $binfile The gdb_compile and clean_restart should be replaced with a single prepare_for_testing call. > +if { ![runto_main] } { > + return > +} > + > +# put breakpoint on each test-function > +foreach func $tests { > + gdb_test "break $func" "Breakpoint.*at.*" "set breakpoint on $func" You can probably use gdb_breakpoint here. But, do you even need these breakpoints? Maybe I'm missing something, but you just seem to runto_main, and then step through everything until the inferior exits. That said, you might be interested to read gdb.base/step-through-epilogue.exp, which does a similar "step until we reach some stopping point" type testing. In that case the whole stepping logic is done in a single gdb_test_multiple. You could do something similar: 1. Setup all the breakpoints as you currently do, 2. Continue until you hit one of these breakpoints, 3. Step until you return to main, 4. If we have more tests to run then goto #2, otherwise we're done. > +} > + > +set rt [single_step] > +if { $rt == 0 } { > + fail "single_step returned $rt" > + return > +} > + > +set start [timestamp] > +global timeout Don't read timeout directly. Use the result of get_largest_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" > +# } Remove comment out code please. > + > + set rt [single_step] > +} > + > +if {$rt == 0 } { > + fail "stepi" > +} It would be good practice to use the same string for all the pass/fail calls within the while loop, and in this final trailing check, these really are the same one test, which either passes or fails. Thanks, Andrew