From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender-0.a4lg.com [IPv6:2401:2500:203:30b:4000:6bfe:4757:0]) by sourceware.org (Postfix) with ESMTPS id 77D213858D38 for ; Sat, 1 Oct 2022 08:15:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 77D213858D38 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 72C8F300089; Sat, 1 Oct 2022 08:15:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=irq.a4lg.com; s=2017s01; t=1664612105; bh=NI4j2i5OpfLQbCRbwLNQwXZO4o5IIn8BXOFeceqwP0c=; h=Message-ID:Date:Mime-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=oA62Iae8EVFbZIIFsUhLANCr/HVUGn53kqt/HzwOrsQNB4FRkTnxllrkZNaOwfBzp i9UotUOIbgpNBeCT9QfpPJjs5hTnuUTpxRNfU7WsLnNvAeJtQ7lA8uSoXlx36u17bP 9qckJjiTZyQ4wqX6+mzZ4ii55NGcf1cL+cC98tY4= Message-ID: Date: Sat, 1 Oct 2022 17:15:03 +0900 Mime-Version: 1.0 Subject: Re: [PATCH 1/1] RISC-V: Move supervisor instructions after all unprivileged ones Content-Language: en-US To: Nelson Chu Cc: binutils@sourceware.org References: <8db04962aba9c780f133840a8934353a58f223fe.1664602716.git.research_trasio@irq.a4lg.com> From: Tsukasa OI In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,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: On 2022/10/01 16:45, Nelson Chu wrote: > On Sat, Oct 1, 2022 at 1:41 PM Tsukasa OI via Binutils > wrote: >> >> This location of supervisor instructions is out of place (because many other >> privileged instructions are located at the tail but after the supervisor >> instructions, we have many unprivileged instructions including bit >> manipulation / crypto / vector instructions). >> >> Not only that, this is harmful to implement pseudoinstructions in the latest >> 'P'-extension proposal (CLROV and RDOV). This commit moves supervisor >> instructions after all unprivileged instructions. > > OK, since I don't have a strong opinion on this. > > Not to reject this patch, just a minor thought - we cannot always > gather all extensions nearly, once some of them support the > instructions based on others. So maybe just place CLROV and RDOV > before csrrs and csrrci is the easier way to do. I agree that we cannot always gather all extensions. Sometimes, we will have to do just like you mentioned (on some instructions). The reason of moving this section is, we can satisfy all of them by doing this: (1) Modification is self-explanatory (simple). (2) Instruction order fits better. (3) Placing CLROV and RDOV just before CSR instructions is no longer needed after doing this. Note that I expect that 'P' instructions are placed before vector instructions. I submitted this patch because it can be a prerequisite for my version of 'P'-extension implementation proposal but this idea is also applicable to another topic: standard hints. I think moving all standard hints (as a separate section) before any standard instructions might be helpful since we don't need to taint standard RVI instruction section by minor (yet standard) hints. Thanks, Tsukasa > > Thanks > Nelson > >> opcodes/ChangeLog: >> >> * riscv-opc.c (riscv_opcodes): Adjust indents. Move supervisor >> instructions after all unprivileged instructions. >> --- >> opcodes/riscv-opc.c | 64 ++++++++++++++++++++++----------------------- >> 1 file changed, 32 insertions(+), 32 deletions(-) >> >> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c >> index 7e95f645c5c..3eb69cdc34c 100644 >> --- a/opcodes/riscv-opc.c >> +++ b/opcodes/riscv-opc.c >> @@ -912,38 +912,6 @@ const struct riscv_opcode riscv_opcodes[] = >> {"c.fswsp", 32, INSN_CLASS_F_AND_C, "CT,CM(Cc)", MATCH_C_FSWSP, MASK_C_FSWSP, match_opcode, INSN_DREF|INSN_4_BYTE }, >> {"c.fsw", 32, INSN_CLASS_F_AND_C, "CD,Ck(Cs)", MATCH_C_FSW, MASK_C_FSW, match_opcode, INSN_DREF|INSN_4_BYTE }, >> >> -/* Supervisor instructions. */ >> -{"csrr", 0, INSN_CLASS_ZICSR,"d,E", MATCH_CSRRS, MASK_CSRRS|MASK_RS1, match_opcode, INSN_ALIAS }, >> -{"csrw", 0, INSN_CLASS_ZICSR,"E,s", MATCH_CSRRW, MASK_CSRRW|MASK_RD, match_opcode, INSN_ALIAS }, >> -{"csrw", 0, INSN_CLASS_ZICSR,"E,Z", MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS }, >> -{"csrwi", 0, INSN_CLASS_ZICSR,"E,Z", MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS }, >> -{"csrs", 0, INSN_CLASS_ZICSR,"E,s", MATCH_CSRRS, MASK_CSRRS|MASK_RD, match_opcode, INSN_ALIAS }, >> -{"csrs", 0, INSN_CLASS_ZICSR,"E,Z", MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS }, >> -{"csrsi", 0, INSN_CLASS_ZICSR,"E,Z", MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS }, >> -{"csrc", 0, INSN_CLASS_ZICSR,"E,s", MATCH_CSRRC, MASK_CSRRC|MASK_RD, match_opcode, INSN_ALIAS }, >> -{"csrc", 0, INSN_CLASS_ZICSR,"E,Z", MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS }, >> -{"csrci", 0, INSN_CLASS_ZICSR,"E,Z", MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS }, >> -{"csrrw", 0, INSN_CLASS_ZICSR,"d,E,s", MATCH_CSRRW, MASK_CSRRW, match_opcode, 0 }, >> -{"csrrw", 0, INSN_CLASS_ZICSR,"d,E,Z", MATCH_CSRRWI, MASK_CSRRWI, match_opcode, INSN_ALIAS }, >> -{"csrrwi", 0, INSN_CLASS_ZICSR,"d,E,Z", MATCH_CSRRWI, MASK_CSRRWI, match_opcode, 0 }, >> -{"csrrs", 0, INSN_CLASS_ZICSR,"d,E,s", MATCH_CSRRS, MASK_CSRRS, match_opcode, 0 }, >> -{"csrrs", 0, INSN_CLASS_ZICSR,"d,E,Z", MATCH_CSRRSI, MASK_CSRRSI, match_opcode, INSN_ALIAS }, >> -{"csrrsi", 0, INSN_CLASS_ZICSR,"d,E,Z", MATCH_CSRRSI, MASK_CSRRSI, match_opcode, 0 }, >> -{"csrrc", 0, INSN_CLASS_ZICSR,"d,E,s", MATCH_CSRRC, MASK_CSRRC, match_opcode, 0 }, >> -{"csrrc", 0, INSN_CLASS_ZICSR,"d,E,Z", MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS }, >> -{"csrrci", 0, INSN_CLASS_ZICSR,"d,E,Z", MATCH_CSRRCI, MASK_CSRRCI, match_opcode, 0 }, >> -{"uret", 0, INSN_CLASS_I, "", MATCH_URET, MASK_URET, match_opcode, 0 }, >> -{"sret", 0, INSN_CLASS_I, "", MATCH_SRET, MASK_SRET, match_opcode, 0 }, >> -{"hret", 0, INSN_CLASS_I, "", MATCH_HRET, MASK_HRET, match_opcode, 0 }, >> -{"mret", 0, INSN_CLASS_I, "", MATCH_MRET, MASK_MRET, match_opcode, 0 }, >> -{"dret", 0, INSN_CLASS_I, "", MATCH_DRET, MASK_DRET, match_opcode, 0 }, >> -{"sfence.vm", 0, INSN_CLASS_I, "", MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 }, >> -{"sfence.vm", 0, INSN_CLASS_I, "s", MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 }, >> -{"sfence.vma", 0, INSN_CLASS_I, "", MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS }, >> -{"sfence.vma", 0, INSN_CLASS_I, "s", MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS2, match_opcode, INSN_ALIAS }, >> -{"sfence.vma", 0, INSN_CLASS_I, "s,t", MATCH_SFENCE_VMA, MASK_SFENCE_VMA, match_opcode, 0 }, >> -{"wfi", 0, INSN_CLASS_I, "", MATCH_WFI, MASK_WFI, match_opcode, 0 }, >> - >> /* Zicbom and Zicboz instructions. */ >> {"cbo.clean", 0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_CLEAN, MASK_CBO_CLEAN, match_opcode, 0 }, >> {"cbo.flush", 0, INSN_CLASS_ZICBOM, "0(s)", MATCH_CBO_FLUSH, MASK_CBO_FLUSH, match_opcode, 0 }, >> @@ -1830,6 +1798,38 @@ const struct riscv_opcode riscv_opcodes[] = >> {"vmv4r.v", 0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV4RV, MASK_VMV4RV, match_opcode, 0}, >> {"vmv8r.v", 0, INSN_CLASS_V, "Vd,Vt", MATCH_VMV8RV, MASK_VMV8RV, match_opcode, 0}, >> >> +/* Supervisor instructions. */ >> +{"csrr", 0, INSN_CLASS_ZICSR, "d,E", MATCH_CSRRS, MASK_CSRRS|MASK_RS1, match_opcode, INSN_ALIAS }, >> +{"csrw", 0, INSN_CLASS_ZICSR, "E,s", MATCH_CSRRW, MASK_CSRRW|MASK_RD, match_opcode, INSN_ALIAS }, >> +{"csrw", 0, INSN_CLASS_ZICSR, "E,Z", MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS }, >> +{"csrwi", 0, INSN_CLASS_ZICSR, "E,Z", MATCH_CSRRWI, MASK_CSRRWI|MASK_RD, match_opcode, INSN_ALIAS }, >> +{"csrs", 0, INSN_CLASS_ZICSR, "E,s", MATCH_CSRRS, MASK_CSRRS|MASK_RD, match_opcode, INSN_ALIAS }, >> +{"csrs", 0, INSN_CLASS_ZICSR, "E,Z", MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS }, >> +{"csrsi", 0, INSN_CLASS_ZICSR, "E,Z", MATCH_CSRRSI, MASK_CSRRSI|MASK_RD, match_opcode, INSN_ALIAS }, >> +{"csrc", 0, INSN_CLASS_ZICSR, "E,s", MATCH_CSRRC, MASK_CSRRC|MASK_RD, match_opcode, INSN_ALIAS }, >> +{"csrc", 0, INSN_CLASS_ZICSR, "E,Z", MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS }, >> +{"csrci", 0, INSN_CLASS_ZICSR, "E,Z", MATCH_CSRRCI, MASK_CSRRCI|MASK_RD, match_opcode, INSN_ALIAS }, >> +{"csrrw", 0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRW, MASK_CSRRW, match_opcode, 0 }, >> +{"csrrw", 0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRWI, MASK_CSRRWI, match_opcode, INSN_ALIAS }, >> +{"csrrwi", 0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRWI, MASK_CSRRWI, match_opcode, 0 }, >> +{"csrrs", 0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRS, MASK_CSRRS, match_opcode, 0 }, >> +{"csrrs", 0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRSI, MASK_CSRRSI, match_opcode, INSN_ALIAS }, >> +{"csrrsi", 0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRSI, MASK_CSRRSI, match_opcode, 0 }, >> +{"csrrc", 0, INSN_CLASS_ZICSR, "d,E,s", MATCH_CSRRC, MASK_CSRRC, match_opcode, 0 }, >> +{"csrrc", 0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRCI, MASK_CSRRCI, match_opcode, INSN_ALIAS }, >> +{"csrrci", 0, INSN_CLASS_ZICSR, "d,E,Z", MATCH_CSRRCI, MASK_CSRRCI, match_opcode, 0 }, >> +{"uret", 0, INSN_CLASS_I, "", MATCH_URET, MASK_URET, match_opcode, 0 }, >> +{"sret", 0, INSN_CLASS_I, "", MATCH_SRET, MASK_SRET, match_opcode, 0 }, >> +{"hret", 0, INSN_CLASS_I, "", MATCH_HRET, MASK_HRET, match_opcode, 0 }, >> +{"mret", 0, INSN_CLASS_I, "", MATCH_MRET, MASK_MRET, match_opcode, 0 }, >> +{"dret", 0, INSN_CLASS_I, "", MATCH_DRET, MASK_DRET, match_opcode, 0 }, >> +{"sfence.vm", 0, INSN_CLASS_I, "", MATCH_SFENCE_VM, MASK_SFENCE_VM | MASK_RS1, match_opcode, 0 }, >> +{"sfence.vm", 0, INSN_CLASS_I, "s", MATCH_SFENCE_VM, MASK_SFENCE_VM, match_opcode, 0 }, >> +{"sfence.vma", 0, INSN_CLASS_I, "", MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS1|MASK_RS2, match_opcode, INSN_ALIAS }, >> +{"sfence.vma", 0, INSN_CLASS_I, "s", MATCH_SFENCE_VMA, MASK_SFENCE_VMA|MASK_RS2, match_opcode, INSN_ALIAS }, >> +{"sfence.vma", 0, INSN_CLASS_I, "s,t", MATCH_SFENCE_VMA, MASK_SFENCE_VMA, match_opcode, 0 }, >> +{"wfi", 0, INSN_CLASS_I, "", MATCH_WFI, MASK_WFI, match_opcode, 0 }, >> + >> /* Svinval instructions. */ >> {"sinval.vma", 0, INSN_CLASS_SVINVAL, "s,t", MATCH_SINVAL_VMA, MASK_SINVAL_VMA, match_opcode, 0 }, >> {"sfence.w.inval", 0, INSN_CLASS_SVINVAL, "", MATCH_SFENCE_W_INVAL, MASK_SFENCE_W_INVAL, match_opcode, 0 }, >> -- >> 2.34.1 >> >