From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id iMWBK73gUGddMwQAWB0awg (envelope-from ) for ; Wed, 04 Dec 2024 18:07:41 -0500 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=dXlk3tX5; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id AEBDD1E197; Wed, 4 Dec 2024 18:07:41 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.0 (2022-12-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable autolearn_force=no version=4.0.0 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id EFD461E05C for ; Wed, 4 Dec 2024 18:07:40 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 9DBB93858D21 for ; Wed, 4 Dec 2024 23:07:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9DBB93858D21 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=dXlk3tX5 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id C08083858D20 for ; Wed, 4 Dec 2024 23:07:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C08083858D20 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 C08083858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733353625; cv=none; b=YxcLHmPlxAeHyzuMaSGk4z3PvAmGRNsYndd3Ac/LoLbzA9jlGZDXRh/hFZxllxuI21Tr775aLbg8ssfOd2RVQux5raaF/mNFsHac7wqblrDg5tsqK9QZ8YLa9Y4TEha+7b6Svjd0awybWeF2uA7octs90IavEWDGm6qnBd0DNzs= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1733353625; c=relaxed/simple; bh=PJeZ7BQKXqKnHFvDzi2JRYBwv6IKGLNVy8aUkHnW/ME=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=hiAWnHjNCApxMGhO+WpjkfW1Ajh7ybPfhP1uhnz2L1UFgiAOnsybeyMihOB5FtWhUBmsswcXatp4UAw/XgzCjkBmbOEgmx3YH+BliBJasLmeBNcNPD+cz0eiwTyX34KI8gHnYqDuBlApgxd3OLl532w0T8BhyFdlokT8rGTU6ao= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C08083858D20 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1733353625; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fsX+pZpNPVkbMk50QM7rjgxrjoBwimcS5zAXbNebw2w=; b=dXlk3tX5gWNBfJ5GdDsG6dlwLYVgzhAmIRij/H4yPVMKGV1ff0ggevQXsoMIIzsmVzElGT BN4S0Lnn12RWmXm2r5I01pXx9Ei3e9B4aTqpLQaa6IFqhJglVBw9/VdKAyBHX6UhKYNlBL G7Dv8MXMkgPM2pmMUOHRYshDrrXrmPk= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-326-tOE_iUM5McCIkwEk7k0k8w-1; Wed, 04 Dec 2024 18:07:02 -0500 X-MC-Unique: tOE_iUM5McCIkwEk7k0k8w-1 X-Mimecast-MFC-AGG-ID: tOE_iUM5McCIkwEk7k0k8w Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 35CDC19560AB; Wed, 4 Dec 2024 23:07:00 +0000 (UTC) Received: from f40-zbm-amd (unknown [10.22.80.92]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 4BB331956054; Wed, 4 Dec 2024 23:06:57 +0000 (UTC) Date: Wed, 4 Dec 2024 16:06:52 -0700 From: Kevin Buettner To: Milica Matic Cc: gdb-patches@sourceware.org, Djordje Todorovic , "dragan.mladjenovic@htecgroup.com" , Milos Kalicanin , "aburgess@redhat.com" , "simark@simark.ca" , "macro@orcam.me.uk" , "cfu@wavecomp.com" Subject: Re: [PATCH^6] gdb: mips: Add MIPSR6 support Message-ID: <20241204160652.2143010e@f40-zbm-amd> In-Reply-To: References: Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: JS9lYebOpI_hlhVapaLHKYw2DKFtgENsT5XJwq6dbcw_1733353620 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Hi Milica, I don't claim to know anything about MIPSR6, but I do see some things which ought to be fixed before this patch goes in. See below... On Fri, 8 Nov 2024 19:20:14 +0000 Milica Matic 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 v5: Sync with current master. > > [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-05-15 > 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. It's okay to include ChangeLog entries as part of the commit log if you want, but you should not try to place these in any of GDB's ChangeLog files. (The GDB project doesn't use ChangeLog files any longer, though you'll still see ChangeLog-YYYY. These are the old ChangeLog files.) My preference is to describe the highlights of what's changed and why it's changed the way it is in the commit log. For your commit, you might describe new functions and how they're used by existing code, but you can probably avoid mentioning new defines. > --- > gdb/mips-tdep.c | 534 +++++++++++++-- > gdb/testsuite/gdb.arch/mips-64-r6.c | 918 ++++++++++++++++++++++++++ > gdb/testsuite/gdb.arch/mips-64-r6.exp | 76 +++ > 3 files changed, 1484 insertions(+), 44 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 c00efbd02ad..173bfe0b8f8 100644 > --- a/gdb/mips-tdep.c > +++ b/gdb/mips-tdep.c > @@ -76,6 +76,10 @@ static int mips16_insn_at_pc_has_delay_slot (struct gdbarch *gdbarch, > static void mips_print_float_info (struct gdbarch *, struct ui_file *, > _____________________________________________________________________ const frame_info_ptr &, const char *); > > +static void > +mips_read_fp_register_single (const frame_info_ptr &frame, int regno, > + gdb_byte *rare_buffer); > + > /* 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) > @@ -325,6 +329,16 @@ mips_abi_regsize (struct gdbarch *gdbarch) > } > } > > +/* 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); > +} > + > /* MIPS16/microMIPS function addresses are odd (bit 0 is set). Here > are some functions to handle addresses associated with compressed > code including but not limited to testing, setting, or clearing > @@ -1496,6 +1510,7 @@ mips_fetch_instruction (struct gdbarch *gdbarch, > return extract_unsigned_integer (buf, instlen, byte_order); > } > > + You've introduced an extraneous blank line here. > /* These are the fields of 32 bit mips instructions. */ > #define mips32_op(x) (x >> 26) > #define itype_op(x) (x >> 26) > @@ -1538,6 +1553,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) > @@ -1574,6 +1590,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; > +} Please add brief descriptive comments before each of the above new functions. (I realize that there may be existing functions in mips-tdep.c which do not have such comments. It'd be nice if they did, but you're only responsible for documenting new or changed functions.) > + > /* 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. */ > @@ -1632,6 +1660,71 @@ is_octeon_bbit_op (int op, struct gdbarch *gdbarch) > return 0; > } > > +static int > +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 int > +is_add64bit_overflow (int64_t a, int64_t b) > +{ > + if (a != (int32_t)a) > + return 1; > + if (b != (int32_t)b) > + return 1; > + return is_add32bit_overflow ((int32_t)a, (int32_t)b); > +} Likewise for the above two functions. > + > +/* Calculate address of next instruction after BLEZ. */ > + > +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); > + int taken = 0; > + 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. */ > @@ -1642,12 +1735,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; > + > inst = mips_fetch_instruction (gdbarch, ISA_MIPS, pc, NULL); > op = itype_op (inst); > if ((inst & 0xe0000000) != 0)______________________________/* Not a special, jump or branch > _______________________________________________________________________________________ instruction. */ For the above two lines and a bunch below, my email client replaces non-UTF characters with '_'. (Or something like that - it's an annoyance to me, but in this case it seems to have flagged that something funny is going on.) Check your local sources and make sure that tabs or spaces are used here. It may just be that your email client changed some things without your knowledge. If you use "git send-email", these problems will likely be avoided. (I've stopped quoting the patch here, because I didn't notice anything further that I should comment on.) Kevin