From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender.a4lg.com [153.120.152.154]) by sourceware.org (Postfix) with ESMTPS id 220FE3858D33 for ; Tue, 8 Aug 2023 06:42:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 220FE3858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=irq.a4lg.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=irq.a4lg.com Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id A8B86300089; Tue, 8 Aug 2023 06:42:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=irq.a4lg.com; s=2017s01; t=1691476947; bh=1GveZ3Jz/pL2E9dK3rK4dOlV+5ba4/iyNhz8Pt8SB1M=; h=Message-ID:Date:Mime-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=VClGsQvkZIq/1GXCtAhTv16d9XtrdT9A86L4adElOWvxsNUsIc2shBLp6m4Dofc7h rN5g00/0teTqpTv1XKbBb7+OMfFd0ZETT1q/Uvwa20x0KE70FpOLT44EI1ONBG0A4E mH2QL9QySTfJJ+N8wtLKo69dTIu8mzVc3V2ckRQI= Message-ID: Date: Tue, 8 Aug 2023 15:42:25 +0900 Mime-Version: 1.0 Subject: Re: [PATCH] RISC-V: Fix opcode entries of "vmsge{,u}.vx" Content-Language: en-US To: Palmer Dabbelt , Andrew Waterman , Jim Wilson , Nelson Chu , Kito Cheng , Jiawei Cc: binutils@sourceware.org References: From: Tsukasa OI In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,KAM_MANYTO,SPF_HELO_NONE,SPF_PASS,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: A little more about how important this fix is. vmsge{,u}.vx matches wide range of compressed instructions and possibly affect wide range of compressed extensions. - xxxx xxxx xxx0 00x0 (vmsge.vx; M_VMSGE == 29) - xxxx xxxx xxx0 000x (vmsgeu.vx; M_VMSGEU == 30) The reason why we don't see any wrong disassembly is detailed in the patch message. And I found that the 'Zcmp' extension patch (a work by Jiawei) actually breaks the disassembler. This is not Jiawei's fault but the consequence of this bug (I want to fix). How to reproduce: 1. Apply and optionally fix Jiawei's Zcmp patchset (applying only push/pop patch is sufficient to reproduce) 2. Assemble "cm.push {ra}, -16" with -march=rv32if_zcmp_zve32f 3. Disassemble the output and we see "vmsge.vx v16,v0,ra,v0.t", not "cm.push {ra}, -16". This is because "cm.push {ra}, -16" mismatches as follows: - 1011 1000 0100 0010 (cm.push {ra}, -16) - xxxx xxxx xxx0 00x0 (wrong matching pattern of vmsge.vx) Note that "cm.push" entry on riscv_opcodes is placed after "vmsge.vx". I don't recommend Jiawei to move "cm.push" riscv_opcode entry because there's no good reason to do it. I recognize that fixing this bug is practically a requirement of adopting the 'Zcmp' / 'Zcmt' extensions (not absolute requirement though) and hope that this bug fix patch is reviewed soon. Sincerely, Tsukasa On 2023/08/06 10:53, Tsukasa OI wrote: > From: Tsukasa OI > > Their check_func should be "match_never", not "match_opcode". The reasons > this error did not cause any disassembler errors are: > > 1. The problem will not reproduce if "no-aliases" is specified > (because macro instructions are handled as aliases). > 2. If not, all affected compressed instructions or their aliases > precede before "vmsge{,u}.vx" macro instructions. > > However, it'll easily break if we reorder opcode entries. This commit > fixes this issue before the *accident* occurs. > > opcodes/ChangeLog: > > * riscv-opc.c (riscv_opcodes): Make sure that we never match to > vmsge{,u}.vx instructions unless specified in the assembler. > --- > opcodes/riscv-opc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c > index 6a854736fec0..37c7694999ad 100644 > --- a/opcodes/riscv-opc.c > +++ b/opcodes/riscv-opc.c > @@ -1606,10 +1606,10 @@ const struct riscv_opcode riscv_opcodes[] = > {"vmsgeu.vi", 0, INSN_CLASS_V, "Vd,Vu,0Vm", MATCH_VMSEQVV, MASK_VMSEQVV, match_vs1_eq_vs2, INSN_ALIAS }, > {"vmsgeu.vi", 0, INSN_CLASS_V, "Vd,Vt,VkVm", MATCH_VMSGTUVI, MASK_VMSGTUVI, match_opcode, INSN_ALIAS }, > > -{"vmsge.vx", 0, INSN_CLASS_V, "Vd,Vt,sVm", 0, (int) M_VMSGE, match_opcode, INSN_MACRO }, > -{"vmsge.vx", 0, INSN_CLASS_V, "Vd,Vt,s,VM,VT", 0, (int) M_VMSGE, match_opcode, INSN_MACRO }, > -{"vmsgeu.vx", 0, INSN_CLASS_V, "Vd,Vt,sVm", 0, (int) M_VMSGEU, match_opcode, INSN_MACRO }, > -{"vmsgeu.vx", 0, INSN_CLASS_V, "Vd,Vt,s,VM,VT", 0, (int) M_VMSGEU, match_opcode, INSN_MACRO }, > +{"vmsge.vx", 0, INSN_CLASS_V, "Vd,Vt,sVm", 0, (int) M_VMSGE, match_never, INSN_MACRO }, > +{"vmsge.vx", 0, INSN_CLASS_V, "Vd,Vt,s,VM,VT", 0, (int) M_VMSGE, match_never, INSN_MACRO }, > +{"vmsgeu.vx", 0, INSN_CLASS_V, "Vd,Vt,sVm", 0, (int) M_VMSGEU, match_never, INSN_MACRO }, > +{"vmsgeu.vx", 0, INSN_CLASS_V, "Vd,Vt,s,VM,VT", 0, (int) M_VMSGEU, match_never, INSN_MACRO }, > > {"vminu.vv", 0, INSN_CLASS_V, "Vd,Vt,VsVm", MATCH_VMINUVV, MASK_VMINUVV, match_opcode, 0}, > {"vminu.vx", 0, INSN_CLASS_V, "Vd,Vt,sVm", MATCH_VMINUVX, MASK_VMINUVX, match_opcode, 0}, > > base-commit: 5e66f55c62e306afbcc93856bf06e542ddd00997