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 330D7385380C for ; Sat, 6 May 2023 23:11:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 330D7385380C 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 A13AB92009C; Sun, 7 May 2023 01:11:32 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by angie.orcam.me.uk (Postfix) with ESMTP id 9AB6992009B; Sun, 7 May 2023 00:11:32 +0100 (BST) Date: Sun, 7 May 2023 00:11:32 +0100 (BST) From: "Maciej W. Rozycki" To: david@davidgf.es cc: binutils@sourceware.org Subject: Re: [PATCH 3/3] Adding more instructions to Allegrex CPU In-Reply-To: <20230328230249.274759-4-david@davidgf.es> Message-ID: References: <20230328230249.274759-1-david@davidgf.es> <20230328230249.274759-4-david@davidgf.es> 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.5 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_ASCII_DIVIDERS,KAM_DMARC_STATUS,KAM_INFOUSMEBIZ,KAM_LAZY_DOMAIN_SECURITY,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 Wed, 29 Mar 2023, david@davidgf.es wrote: > From: David Guillen Fandos > > Many instructions exist already in MIPS32, however some have different > encodings. Some are new (ie. wsbw, min, max) and might be very device Two spaces after the full stop here please. > diff --git a/opcodes/mips-opc.c b/opcodes/mips-opc.c > index 5dd6c8f51e..1c6cd567d4 100644 > --- a/opcodes/mips-opc.c > +++ b/opcodes/mips-opc.c > @@ -993,8 +993,10 @@ const struct mips_opcode mips_builtin_opcodes[] = > {"cins", "t,r,+p,+S", 0x70000032, 0xfc00003f, WR_1|RD_2, 0, IOCT, 0, 0 }, > {"clo", "d,s", 0x00000051, 0xfc1f07ff, WR_1|RD_2, 0, I37, 0, 0 }, > {"clo", "U,s", 0x70000021, 0xfc0007ff, WR_1|RD_2, 0, I32|N55, 0, I37 }, > +{"clo", "d,s", 0x00000017, 0xfc1f07ff, WR_1|RD_2, 0, AL, 0, 0 }, > {"clz", "d,s", 0x00000050, 0xfc1f07ff, WR_1|RD_2, 0, I37, 0, 0 }, > {"clz", "U,s", 0x70000020, 0xfc0007ff, WR_1|RD_2, 0, I32|N55, 0, I37 }, > +{"clz", "d,s", 0x00000016, 0xfc1f07ff, WR_1|RD_2, 0, AL, 0, 0 }, As a lower ISA implementation please put these new entries first rather than last (similarly to "msub" and "msubu" below). > @@ -2129,7 +2136,8 @@ const struct mips_opcode mips_builtin_opcodes[] = > {"addu_s.ob", "d,s,t", 0x7c000114, 0xfc0007ff, WR_1|RD_2|RD_3, 0, 0, D64, 0 }, > {"addu_s.qb", "d,s,t", 0x7c000110, 0xfc0007ff, WR_1|RD_2|RD_3, 0, 0, D32, 0 }, > {"addwc", "d,s,t", 0x7c000450, 0xfc0007ff, WR_1|RD_2|RD_3, 0, 0, D32, 0 }, > -{"bitrev", "d,t", 0x7c0006d2, 0xffe007ff, WR_1|RD_2, 0, 0, D32, 0 }, > +{"bitrev", "d,w", 0x7c000520, 0xffe007ff, WR_1|RD_2, 0, AL, 0, 0 }, > +{"bitrev", "d,t", 0x7c0006d2, 0xffe007ff, WR_1|RD_2, 0, 0, D32, AL }, Why is the AL exclusion actually needed with the second entry here? I'd have thought the DSP ASE requirement would keep the entry unavailable for Allegrex. > diff --git a/gas/testsuite/gas/mips/allegrex.d b/gas/testsuite/gas/mips/allegrex.d > new file mode 100644 > index 0000000000..89c2d092ad > --- /dev/null > +++ b/gas/testsuite/gas/mips/allegrex.d > @@ -0,0 +1,49 @@ > +#as: -march=allegrex -mabi=32 > +#objdump: -dr --prefix-addresses --show-raw-insn -M reg-names=numeric > +#name: Sony Allegrex CPU tests > + > +.*: file format .* > + > + > +Disassembly of section .text: [...] > +0x000000a0 7000003e dret This test case fails for some configurations due to a difference in section alignment between MIPS targets causing a different amount of padding to be added at the end to satisfy alignment: extra lines in tmpdir/dump.out starting with "^ ...$" EOF from .../gas/testsuite/gas/mips/allegrex.d FAIL: Sony Allegrex CPU tests One way to address this problem is to add padding by hand, such as with the patch below. We use this across many test cases already. Otherwise this is OK. Maciej --- gas/testsuite/gas/mips/allegrex.d | 1 + gas/testsuite/gas/mips/allegrex.s | 3 +++ 2 files changed, 4 insertions(+) Index: binutils-gdb/gas/testsuite/gas/mips/allegrex.d =================================================================== --- binutils-gdb.orig/gas/testsuite/gas/mips/allegrex.d +++ binutils-gdb/gas/testsuite/gas/mips/allegrex.d @@ -47,3 +47,4 @@ 0x00000098 7002083d mfdr \$2,\$1 0x0000009c 7083083d mtdr \$3,\$1 0x000000a0 7000003e dret + \.\.\. Index: binutils-gdb/gas/testsuite/gas/mips/allegrex.s =================================================================== --- binutils-gdb.orig/gas/testsuite/gas/mips/allegrex.s +++ binutils-gdb/gas/testsuite/gas/mips/allegrex.s @@ -42,3 +42,6 @@ mtdr $v1, $1 dret +# Force some (non-delay-slot) zero bytes, to make 'objdump' print ... + .align 4, 0 + .space 16