From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x2b.google.com (mail-oa1-x2b.google.com [IPv6:2001:4860:4864:20::2b]) by sourceware.org (Postfix) with ESMTPS id CC72D385B524 for ; Thu, 14 Dec 2023 03:05:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org CC72D385B524 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org CC72D385B524 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2001:4860:4864:20::2b ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702523158; cv=none; b=Hby/fBLnOUefmniNOk6oRZu8VWHkdtJIS74hs6Dh2uYlAo3AI+vZfuxMbaLoIoDfh4Yh5gU5V630mjwJWGRA4JO7hjfRVkeGy2AEHb9xTfjDaEbOzMJo4viT03sbGZxbUF7nEefFPsESf6r9S173F1AwSYJLRCYCyPmzX2BwHL8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702523158; c=relaxed/simple; bh=njil22eeUioUlcQ505CfWDs72CGOufBcrwE5cATDxQs=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=BiuEchwbPgZuCqRvcyxCAIWT3RmG7/Wj89bV/LjWviR9DFl4d251aHl8z64AJISHet3UPp4+hBa1C/1iqR20tdPAr3W51cLKMd74oHgQmBCXB2sEolXOwmy9KszgU3DQakxTE+Werx4QL/WlkYtMjSEzOScFBvJUoIJ4szqm4xs= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-oa1-x2b.google.com with SMTP id 586e51a60fabf-203287ba408so805622fac.0 for ; Wed, 13 Dec 2023 19:05:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1702523155; x=1703127955; darn=sourceware.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=RVw62fUxBUyT9KPQsv3kLBRzL3wGISJAcHsPARej3Dc=; b=VpWhm8L1CPB27WJsQkR2rktBVM1SqdlgqgHgQXyGFXV6ksDhwoiL7SQKhsJyIHnXMw JzOk8dKgKobyMHVurwgAg9Doi4KDiFeFfDi+EtPgSYMH2mHr66OY3Rb8qlUKfNuVACYs d8BNgvIMmjBxqHYNhG5mr7fZYKlFQFaHYhTL0si6hxkttIuLVD/f2D9kAjpK8GKx/5Eh pI9gbnSP7Kuh+UpDWiNdkUPGnaCMzFIis8G/ZCPrMRDK9Ln3/LFnkPzr86zIycuxmKpL N7dyCsAlxxVtyFzdBhYJ6Rbh+I8o8rBS0n5VYXnqzoyZ9CF5/SQJPHw+z+ZBiA7w8Uu0 AcTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702523155; x=1703127955; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=RVw62fUxBUyT9KPQsv3kLBRzL3wGISJAcHsPARej3Dc=; b=AvsbSB+QSNwwMCu0hPfQ09MweXZZ4akoWMLZbNmL5JPeleUe/ct1dkYb7RB9mshA67 C4DPH+t5eqzBqczL5nXLuCFiWuVQH/n+36hQ4dVCeCA2uo/MgLqwxl+/RR3H10MGuDWz L2v9vwqQq6YTqL/t10pcJuYv2bAAjgc4451yMJ+F8oXH18G6WYvkvhzUmriEKYKUK/C0 z3AFw8OGeA7XJOuJahx/aiLhOyF/pL4KDvRKoBTvvbWEfAdmBfJKr4b24O3TbglcG72D L9zpD1H9kDyTC+n2pkj3uI+d1uGDqLeM6C4JGaTmaRAkvunS5X7QewyTwmkHiU70BWx+ 9rgg== X-Gm-Message-State: AOJu0Yz+SWnoiA6+c3oVtPL15rSdi33iJKMdvx3uFoWeB2+xe8FDfPz4 2Ub0CmHG/5j4dzLgzjylVo2zOhhbuelUxACJhB1aIg== X-Google-Smtp-Source: AGHT+IFQCLTihSv3HFofO12C2vHbgfJcghA8IMGxhTQVOjsdJwiD0Xk/knfzAZNeLGhOxXdH9gBE/bvs2nKIKn/aqCU= X-Received: by 2002:a05:6870:e253:b0:1fb:22f4:c49d with SMTP id d19-20020a056870e25300b001fb22f4c49dmr11857865oac.23.1702523154791; Wed, 13 Dec 2023 19:05:54 -0800 (PST) MIME-Version: 1.0 References: <20231117062053.1873-1-jinma@linux.alibaba.com> In-Reply-To: From: Nelson Chu Date: Thu, 14 Dec 2023 11:05:43 +0800 Message-ID: Subject: Re: [PATCH] RISC-V: Fix the wrong encoding and operand of the XTheadFmv extension. To: =?UTF-8?Q?Christoph_M=C3=BCllner?= Cc: Jin Ma , binutils@sourceware.org, lifang_xia@linux.alibaba.com, jinma.contrib@gmail.com Content-Type: multipart/alternative; boundary="000000000000dd1284060c6f8f35" X-Spam-Status: No, score=-9.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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: --000000000000dd1284060c6f8f35 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thanks for testing the correctness, please commit. Nelson On Wed, Dec 13, 2023 at 3:00=E2=80=AFPM Christoph M=C3=BCllner < christoph.muellner@vrull.eu> wrote: > On Fri, Nov 17, 2023 at 7:21=E2=80=AFAM Jin Ma = wrote: > > > > The description of instructions 'th.fmv.hw.x' and 'th.fmv.x.hw' of the > > XTheadFmv extension in T-Head specific is incorrect, and it also has > > some impact on the implementation of the binutils, so this patch > > corrects this. > > This changes two things: > 1) Fix swapped operand register types of th.fmv.hw.x > 2) Revert of 767e2daed4da3245350d1e45e4eee42964d656f6 > > The swapped operands were likely caused by unclear description in the spe= c. > This got cleaned up in > https://github.com/T-head-Semi/thead-extension-spec/pull/34. > > The encoding change back then was triggered by > https://github.com/T-head-Semi/thead-extension-spec/pull/11. > I assume that this spec change was a mistake (I don't have HW so I > cannot double-check). > > To make sure we are consistent with other projects I checked the followin= g: > * QEMU: we never updated QEMU to follow PR#11, therefore no change needed > now > * GCC: we always emitted the operands in the correct order and correct > type, therefore no change needed now > * Specification: the mnemonics matches Binutils/GCC and the encoding > matches QEMU/Binutils > > After reviewing all relevant pieces, this patch looks good to me. > I've also tested this patch on current master and all tests pass. > > Reviewed-by: Christoph Muellner > Tested-by: Christoph Muellner > > > > > > For details see: > > https://github.com/T-head-Semi/thead-extension-spec/pull/34 > > > > gas/ChangeLog: > > > > * testsuite/gas/riscv/x-thead-fmv.d: Correct test. > > * testsuite/gas/riscv/x-thead-fmv.s: Likewise. > > > > include/ChangeLog: > > > > * opcode/riscv-opc.h (MATCH_TH_FMV_HW_X): Correct coding. > > (MASK_TH_FMV_HW_X): Likewise. > > (MATCH_TH_FMV_X_HW): Likewise. > > (MASK_TH_FMV_X_HW): Likewise. > > > > opcodes/ChangeLog: > > > > * riscv-opc.c: Correct operands. > > --- > > gas/testsuite/gas/riscv/x-thead-fmv.d | 4 ++-- > > gas/testsuite/gas/riscv/x-thead-fmv.s | 2 +- > > include/opcode/riscv-opc.h | 8 ++++---- > > opcodes/riscv-opc.c | 2 +- > > 4 files changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/gas/testsuite/gas/riscv/x-thead-fmv.d > b/gas/testsuite/gas/riscv/x-thead-fmv.d > > index af8ce0c8ee0..50ccc62413f 100644 > > --- a/gas/testsuite/gas/riscv/x-thead-fmv.d > > +++ b/gas/testsuite/gas/riscv/x-thead-fmv.d > > @@ -7,5 +7,5 @@ > > Disassembly of section .text: > > > > 0+000 : > > -[ ]+[0-9a-f]+:[ ]+5005950b[ ]+th.fmv.hw.x[ ]+a0,fa1 > > -[ ]+[0-9a-f]+:[ ]+6005158b[ ]+th.fmv.x.hw[ ]+a1,fa0 > > +[ ]+[0-9a-f]+:[ ]+a005158b[ ]+th.fmv.hw.x[ ]+fa1,a0 > > +[ ]+[0-9a-f]+:[ ]+c005158b[ ]+th.fmv.x.hw[ ]+a1,fa0 > > diff --git a/gas/testsuite/gas/riscv/x-thead-fmv.s > b/gas/testsuite/gas/riscv/x-thead-fmv.s > > index 250ba8358ae..8ca2ec2f093 100644 > > --- a/gas/testsuite/gas/riscv/x-thead-fmv.s > > +++ b/gas/testsuite/gas/riscv/x-thead-fmv.s > > @@ -1,3 +1,3 @@ > > target: > > - th.fmv.hw.x a0, fa1 > > + th.fmv.hw.x fa1, a0 > > th.fmv.x.hw a1, fa0 > > diff --git a/include/opcode/riscv-opc.h b/include/opcode/riscv-opc.h > > index 24217062edc..061e35d8603 100644 > > --- a/include/opcode/riscv-opc.h > > +++ b/include/opcode/riscv-opc.h > > @@ -2515,10 +2515,10 @@ > > #define MATCH_TH_FSURW 0x5000700b > > #define MASK_TH_FSURW 0xf800707f > > /* Vendor-specific (T-Head) XTheadFmv instructions. */ > > -#define MATCH_TH_FMV_HW_X 0x5000100b > > -#define MASK_TH_FMV_HW_X 0xfff0707f > > -#define MATCH_TH_FMV_X_HW 0x6000100b > > -#define MASK_TH_FMV_X_HW 0xfff0707f > > +#define MATCH_TH_FMV_X_HW 0xc000100b > > +#define MASK_TH_FMV_X_HW 0xfff0707f > > +#define MATCH_TH_FMV_HW_X 0xa000100b > > +#define MASK_TH_FMV_HW_X 0xfff0707f > > /* Vendor-specific (T-Head) XTheadInt instructions. */ > > #define MATCH_TH_IPOP 0x0050000b > > #define MASK_TH_IPOP 0xffffffff > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c > > index 72d727cd77e..3a4ab62d274 100644 > > --- a/opcodes/riscv-opc.c > > +++ b/opcodes/riscv-opc.c > > @@ -2157,7 +2157,7 @@ const struct riscv_opcode riscv_opcodes[] =3D > > {"th.fsurw", 0, INSN_CLASS_XTHEADFMEMIDX, "D,s,t,Xtu2@25", > MATCH_TH_FSURW, MASK_TH_FSURW, match_opcode, 0}, > > > > /* Vendor-specific (T-Head) XTheadFmv instructions. */ > > -{"th.fmv.hw.x", 32, INSN_CLASS_XTHEADFMV, "d,S", MATCH_TH_FMV_HW_X, > MASK_TH_FMV_HW_X, match_opcode, 0}, > > +{"th.fmv.hw.x", 32, INSN_CLASS_XTHEADFMV, "D,s", MATCH_TH_FMV_HW_X, > MASK_TH_FMV_HW_X, match_opcode, 0}, > > {"th.fmv.x.hw", 32, INSN_CLASS_XTHEADFMV, "d,S", MATCH_TH_FMV_X_HW, > MASK_TH_FMV_X_HW, match_opcode, 0}, > > > > /* Vendor-specific (T-Head) XTheadInt instructions. */ > > > > base-commit: 0da4f405f8d9d15b9381075debce788251e31815 > > -- > > 2.17.1 > > > --000000000000dd1284060c6f8f35--