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.133.124]) by sourceware.org (Postfix) with ESMTPS id B1F1F3858D28 for ; Tue, 17 Jan 2023 10:28:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B1F1F3858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673951319; 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=+HwUlv6HBVyLOOenkNUhCNy1bAdFgpkt2X8iQcD2i0Y=; b=H/E/1HchGt+5Atzk/sq5OKN7AIdv9tRInnBsZ2QPXOKivIKNVnYxCaQhIs/YBZ7tos+Ucn 8jujOj+X8mvY/pelWQXcbhJhxRyWwNDkgrFPrkjjJhXgoROXOx3nYmknYXFJ8pJDYgZyYs ntOgu2jyU9PHUl+BTX2DGEfRDUKnyPs= Received: from mail-qk1-f198.google.com (mail-qk1-f198.google.com [209.85.222.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-536-xEWtThUPMxuZzK9yd-v6YA-1; Tue, 17 Jan 2023 05:28:37 -0500 X-MC-Unique: xEWtThUPMxuZzK9yd-v6YA-1 Received: by mail-qk1-f198.google.com with SMTP id bs44-20020a05620a472c00b0070673cd1b05so3094511qkb.22 for ; Tue, 17 Jan 2023 02:28:37 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=+HwUlv6HBVyLOOenkNUhCNy1bAdFgpkt2X8iQcD2i0Y=; b=hE4OxPh3OtTrwNavu5H1JR2iEohg8JTc76BuSpyCQqC7X4Grhn/exoJAknUtFNyjAd JFN5aTWLFNnZbRQr55EdlheKSZUXp2okC/DUZvYVNIhDUmo0SLbYYX3Zit7yVxLhJx8S uwgKOsDRLBHjMWwZKPYXLr82W1kS33yh8rVxeIzRel/08Cwtggh3ZFpS1tuYA/SmO1Kb BlKyPflWkkn/LDxpadIxUyV03krs6NodyiHOtZnO1861e1RcfYGGaBMQOnvlzmbkszZj u+Z3SKW7mN7xjacfGgxY9zhOWLH7i3CIVBFkU7UOz2sOI8GY6uIMw8qSuKtYB4Z605BX RbNg== X-Gm-Message-State: AFqh2konpghz9QhpE0qLYgcwP3GIlk/Wq/Djq1olP8sMv21FSPDAiSgM KQlSsOqdmetpBS8Lop1iVuR7EUrLBlt6XYf0wQVZh2FHbYf6B2Q6Bt+WQO/gEyco73xbmopw6hK hV9DTKrMvhZ87sHYTmA== X-Received: by 2002:ac8:4887:0:b0:3ad:b9af:a507 with SMTP id i7-20020ac84887000000b003adb9afa507mr2612453qtq.52.1673951317232; Tue, 17 Jan 2023 02:28:37 -0800 (PST) X-Google-Smtp-Source: AMrXdXvqbWDhRRHpMYXmlLxKkPezd56ddTH4XuZxUzdELC28ghvlNmChYYaXGYKDEX7T4A+sRPf5nQ== X-Received: by 2002:ac8:4887:0:b0:3ad:b9af:a507 with SMTP id i7-20020ac84887000000b003adb9afa507mr2612440qtq.52.1673951316880; Tue, 17 Jan 2023 02:28:36 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id do26-20020a05620a2b1a00b0070648cf78bdsm5887954qkb.54.2023.01.17.02.28.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 17 Jan 2023 02:28:36 -0800 (PST) From: Andrew Burgess To: "Maciej W. Rozycki" Cc: binutils@sourceware.org Subject: Re: [PATCH 1/2] opcodes/mips: use .word/.short for undefined instructions In-Reply-To: References: <87fscny5tr.fsf@redhat.com> Date: Tue, 17 Jan 2023 10:28:34 +0000 Message-ID: <87a62hfoa5.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.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,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: "Maciej W. Rozycki" writes: > On Fri, 6 Jan 2023, Andrew Burgess wrote: > >> > FYI, I find this questionable as `.word' (at least with the MIPS target) >> > implies natural alignment while 32-bit microMIPS encodings, valid or not, >> > are not. Also, given the endianness peculiarity (analogous to the MIPS16 >> > extended encodings), I think this needs to be ".short\t0x%x, 0x%x" really, >> > with the instruction word split into halfwords for any reasonable meaning. >> > This is already reflected in the raw hex dump of instruction streams; the >> > numbers printed need to match it. >> > >> > With the naked number previously used this obviously didn't matter as it >> > stood out without any attempt to pretend to have a meaning. This is also >> > the reason why I chose to keep it as it used to be since forever. >> >> Below is an initial patch. When I set the environment variable >> DISABLE_MATCHING then the disassembler fails to match all instructions, >> so prints .short for everything. >> >> Right now I can't find anything where this doesn't work, but I don't >> believe that the answer is actually this simple. Given your deeper >> knowledge of the target, could you take a look at what I have below and >> point me at some tests/configurations/whatever where this isn't going to >> be good enough? >> >> Alternatively, if this is enough, then I'll write this up into a proper >> patch. > > Your change is probably right. I'd have thought we have coverage for > this in the testsuite, but perhaps we don't. > > If you try this source code (which uses a reserved 32-bit encoding in the > microMIPS ISA): > > .module micromips > foo: > .insn > .short 0x7f6e, 0x5d4c > > and assemble it for both endiannesses (i.e. with `-EL' and `-EB' passed to > GAS respectively), then I'd expect output like: > > Disassembly of section .text: > > 00000000 : > 0: 7f6e 5d4c .short 0x7f6e, 0x5d4c > ... > > from `objdump -d' in both cases. If this is the case, then the change is > right. > > You can use this example, preferably along with the change itself, for a > testcase to place in binutils/testsuite/binutils-all/mips/. I suggest > using `run_dump_test_o32'/`run_dump_test_n32'/`run_dump_test_n64' all at a > time just as with most of the preexisting test cases just to make sure all > the three BFD backends involved handle this right. > > Let me know if you need further information. Hi Maciej, Sorry for the time taken to prepare this patch. Let me know if you're happy for my to push the below, or if there's anything else that's needed. Thanks, Andrew --- commit 1502245dc2194e8e06a69275b22cffd211218578 Author: Andrew Burgess Date: Fri Jan 6 16:42:23 2023 +0000 opcodes/mips: disassemble unknown micromips instructions as two shorts Before commit: commit 2438b771ee07be19d5b01ea55e077dd8b7cef445 Date: Wed Nov 2 15:53:43 2022 +0000 opcodes/mips: use .word/.short for undefined instructions unknown 32-bit microMIPS instructions were disassembled as a raw 32-bit number with no '.word' directive. The above commit changed this and added a '.word' directive before the 32-bit number. It was pointed out on the mailing list, that for microMIPS it would be better to display such 32-bit instructions using a '.short' directive followed by two 16-bit values. This commit updates the mips disassembler to do this, and adds a new test that validates this output. diff --git a/binutils/testsuite/binutils-all/mips/micromips-reserved-enc.d b/binutils/testsuite/binutils-all/mips/micromips-reserved-enc.d new file mode 100644 index 00000000000..fdcc6deae8f --- /dev/null +++ b/binutils/testsuite/binutils-all/mips/micromips-reserved-enc.d @@ -0,0 +1,9 @@ +#PROG: objcopy +#objdump: -d --prefix-addresses --show-raw-insn +#name: microMIPS source file contains reserved encoding + +.*: +file format .*mips.* + +Disassembly of section \.text: +[0-9a-f]+ <[^>]*> 7f6e 5d4c \.short 0x7f6e, 0x5d4c + \.\.\. diff --git a/binutils/testsuite/binutils-all/mips/micromips-reserved-enc.s b/binutils/testsuite/binutils-all/mips/micromips-reserved-enc.s new file mode 100644 index 00000000000..59113a7980d --- /dev/null +++ b/binutils/testsuite/binutils-all/mips/micromips-reserved-enc.s @@ -0,0 +1,4 @@ + .module micromips +foo: + .insn + .short 0x7f6e, 0x5d4c diff --git a/binutils/testsuite/binutils-all/mips/mips.exp b/binutils/testsuite/binutils-all/mips/mips.exp index 6a0ec25a06f..f43109a75b8 100644 --- a/binutils/testsuite/binutils-all/mips/mips.exp +++ b/binutils/testsuite/binutils-all/mips/mips.exp @@ -266,3 +266,7 @@ run_dump_test_n64 "global-local-symtab-sort-n64${tmips}" run_dump_test_o32 "global-local-symtab-final-o32" useld run_dump_test_n32 "global-local-symtab-final-n32" useld run_dump_test_n64 "global-local-symtab-final-n64" useld + +run_dump_test_o32 "micromips-reserved-enc" +run_dump_test_n32 "micromips-reserved-enc" +run_dump_test_n64 "micromips-reserved-enc" diff --git a/opcodes/mips-dis.c b/opcodes/mips-dis.c index 6a513cd8946..80c35f4a5e0 100644 --- a/opcodes/mips-dis.c +++ b/opcodes/mips-dis.c @@ -2601,11 +2601,19 @@ print_insn_micromips (bfd_vma memaddr, struct disassemble_info *info) } if (length == 2) - infprintf (is, dis_style_assembler_directive, ".short"); + { + infprintf (is, dis_style_assembler_directive, ".short"); + infprintf (is, dis_style_text, "\t"); + infprintf (is, dis_style_immediate, "0x%x", insn); + } else - infprintf (is, dis_style_assembler_directive, ".word"); - infprintf (is, dis_style_text, "\t"); - infprintf (is, dis_style_immediate, "0x%x", insn); + { + infprintf (is, dis_style_assembler_directive, ".short"); + infprintf (is, dis_style_text, "\t"); + infprintf (is, dis_style_immediate, "0x%x", (insn >> 16) & 0xffff); + infprintf (is, dis_style_text, ", "); + infprintf (is, dis_style_immediate, "0x%x", (insn & 0xffff)); + } info->insn_type = dis_noninsn; return length;