From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from angie.orcam.me.uk (angie.orcam.me.uk [IPv6:2001:4190:8020::34]) by sourceware.org (Postfix) with ESMTP id 1C4D83858D35 for ; Mon, 22 May 2023 14:03:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1C4D83858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=orcam.me.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=orcam.me.uk Received: by angie.orcam.me.uk (Postfix, from userid 500) id 691B792009C; Mon, 22 May 2023 16:02:59 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 6598F92009B; Mon, 22 May 2023 15:02:59 +0100 (BST) Date: Mon, 22 May 2023 15:02:59 +0100 (BST) From: "Maciej W. Rozycki" To: Tsing cc: binutils@sourceware.org Subject: Re: [PATCH] Add GINV(+VIRT) ASE for MIPSr6 In-Reply-To: <20230518085739.2195035-1-lei.wang@oss.cipunited.com> Message-ID: References: <20230518085739.2195035-1-lei.wang@oss.cipunited.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-3494.6 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,KAM_INFOUSMEBIZ,KAM_LAZY_DOMAIN_SECURITY,KAM_NUMSUBJECT,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE 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: On Thu, 18 May 2023, Tsing wrote: > This patch adds the VZ extension to MIPSr6 as part of the GINV ASE. > > gas/ChangeLog > * config/tc-mips.c (mips_ases): Add microMIPS ASE. This surely isn't right. > diff --git a/gas/testsuite/gas/mips/ginv-virt.d b/gas/testsuite/gas/mips/ginv-virt.d > new file mode 100644 > index 00000000..50fa6d54 > --- /dev/null > +++ b/gas/testsuite/gas/mips/ginv-virt.d > @@ -0,0 +1,22 @@ > +#objdump: -pdr --prefix-addresses --show-raw-insn > +#name: MIPS GINV Virtualization > +#as: --defsym VX= -mginv -mvirt -32 Please use `VX=1'. I'm surprised using `--defsym' without giving the symbol a value even works. > diff --git a/gas/testsuite/gas/mips/mips.exp b/gas/testsuite/gas/mips/mips.exp > index fafed2dc..4e3e226f 100644 > --- a/gas/testsuite/gas/mips/mips.exp > +++ b/gas/testsuite/gas/mips/mips.exp > @@ -2131,6 +2131,7 @@ if { [istarget mips*-*-vxworks*] } { > > run_dump_test_arches "ginv" [mips_arch_list_matching mips32r6] > run_dump_test_arches "ginv-err" [mips_arch_list_matching mips32r6] > + run_dump_test_arches "ginv-virt" [mips_arch_list_matching mips32r6] Use tabs for indentation. > diff --git a/include/opcode/mips.h b/include/opcode/mips.h > index 75d3fc25..b26a8ffd 100644 > --- a/include/opcode/mips.h > +++ b/include/opcode/mips.h > @@ -1316,6 +1316,9 @@ static const unsigned int mips_isa_table[] = { > /* The Enhanced VA Scheme (EVA) extension has instructions which are > only valid for the R6 ISA. */ > #define ASE_EVA_R6 0x02000000 > +/* The Virtualization ASE has Global INValidate (GINV) > + instructions which are only valid when both ASEs are enabled. */ > +#define ASE_GINV_VIRT 0x08000000 Why is there a hole in bit assignments here? Also use tabs for indentation. > @@ -2351,6 +2354,9 @@ extern const int bfd_mips16_num_opcodes; > "+*" 5-bit register vector element index at bit 16 > "+|" 8-bit mask at bit 16 > > + GINV ASE usage: > + "+\" 2 bit Global TLB invalidate type at bit 8 We write bit-field references with a hyphen, i.e. "2-bit". > diff --git a/opcodes/mips-opc.c b/opcodes/mips-opc.c > index 2a1a6cbb..80a5124b 100644 > --- a/opcodes/mips-opc.c > +++ b/opcodes/mips-opc.c > @@ -412,6 +412,7 @@ decode_mips_operand (const char *p) > > /* Global INValidate (GINV) support. */ > #define GINV ASE_GINV > +#define GINVVZ ASE_GINV_VIRT Use a tab here. > @@ -3329,6 +3330,7 @@ const struct mips_opcode mips_builtin_opcodes[] = > /* MIPS Global INValidate (GINV) ASE. */ > {"ginvi", "s", 0x7c00003d, 0xfc1fffff, RD_1, 0, 0, GINV, 0 }, > {"ginvt", "s,+\\", 0x7c0000bd, 0xfc1ffcff, RD_1, 0, 0, GINV, 0 }, > +{"ginvgt", "s,+\\", 0x7c0000fd, 0xfc1ffcff, RD_1, 0, 0, GINVVZ, 0 }, I take it the encoding is correct, as I can't find it in published docs, and the instruction itself is only mentioned once, in the context of SYNC. How did you verify your change? Maciej