From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x536.google.com (mail-pg1-x536.google.com [IPv6:2607:f8b0:4864:20::536]) by sourceware.org (Postfix) with ESMTPS id B4F2F385800D for ; Wed, 14 Jul 2021 20:38:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B4F2F385800D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dabbelt.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dabbelt.com Received: by mail-pg1-x536.google.com with SMTP id a2so3740938pgi.6 for ; Wed, 14 Jul 2021 13:38:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dabbelt-com.20150623.gappssmtp.com; s=20150623; h=date:subject:in-reply-to:cc:from:to:message-id:mime-version :content-transfer-encoding; bh=o8hcCMry79XlqFsvLJLfMv+xF02N2Oo5za8mfK8y++A=; b=gKc2NRhXxdBnSqDSg8mOc4xaYnNi5JtAGhmSpCX7sbsrbMp7s9EYiAiT97wyjo09MP A7SRt7S5nfy8i1jdlY+4BYN3HdoCSwujfbz/31MP+58uyJ6P+h8C//j9rJiySj9OYzfA 5UmyMMTwyLOE1JR3bKpaRFLS38kh634CvkASRVo0fMwoHK6DAHqHXB4j38xFpkONMhp1 /mLbPQpnjQjHV99vL9o2G8vrTcKcsFZP1ISBf06/cOQcuHFNmu9rJrREpMpU13rm30tO IbyyXcYDRdi++tCn1qlWg837tjB5Uty4N9DwW1WH09pAGEl16Kp79hIrulCxMhRLQVoj x4YA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:subject:in-reply-to:cc:from:to:message-id :mime-version:content-transfer-encoding; bh=o8hcCMry79XlqFsvLJLfMv+xF02N2Oo5za8mfK8y++A=; b=XNRwafGT2tzMbpC+bTazJORmmVKfJWGpD9R96EaltYe8S9yI+AwvPAL4lYhKKvybhG YQ3kPmrSNAPhYLfWsoNJAhC2MBPMfxE75V7qoIV3OxjVnQkjZh9cpDaOUFabsAHcCMKb yVsfdxcIQKw2qahWiLdSrzdW/WWIKqREhplrz5YGuh6D2XjK7SesEcXk/Gh0dshD6GOC pq4lsnxJA0W7llThWAyCbjMzitmDC20KnLEYPzfiQ28jr5uF9SbfBkbMgPQpjSLjeS7U sHBX6uY6olJqfU+rUzLkyBk8DsPq9GUXXaaArJ35UnxnynJcAAcr7/NruabfDjOgk+gv EKFA== X-Gm-Message-State: AOAM530d4EsOxvcgyKqG0P5YkxkHWN3zAOrmliiWFXglOsIoD9lvvlNs 3uCDmHXo9B4uXd/+ZDb53Xl68A== X-Google-Smtp-Source: ABdhPJwgd7FtwmWXA8G1BYPyxCnAyN6vj05zQ6Masy2anKuqqp7IhSIT09PzGgz7wK2y6+Ztk7nNoA== X-Received: by 2002:a63:8f04:: with SMTP id n4mr11081505pgd.317.1626295117734; Wed, 14 Jul 2021 13:38:37 -0700 (PDT) Received: from localhost (76-210-143-223.lightspeed.sntcca.sbcglobal.net. [76.210.143.223]) by smtp.gmail.com with ESMTPSA id x40sm4014755pfu.176.2021.07.14.13.38.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Jul 2021 13:38:37 -0700 (PDT) Date: Wed, 14 Jul 2021 13:38:37 -0700 (PDT) X-Google-Original-Date: Wed, 14 Jul 2021 10:04:54 PDT (-0700) Subject: Re: [PATCH v2 3/3] RISC-V: PR27916, Extend .insn directive to support hardcode encoding. In-Reply-To: CC: nelson.chu@sifive.com, binutils@sourceware.org, gdb-patches@sourceware.org, Jim Wilson , andrew.burgess@embecosm.com, Andrew Waterman From: Palmer Dabbelt To: nelson.chu@sifive.com, kito.cheng@sifive.com Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Jul 2021 20:38:41 -0000 On Fri, 09 Jul 2021 01:01:36 PDT (-0700), kito.cheng@sifive.com wrote: > This patch is extending new formats of .insn, so I suggest this should > be a standalone patch. Ya, that's probably best. In practice it doesn't matter all that much, as it's not like we pull these in via branches or anything. > On Fri, Jul 9, 2021 at 3:28 PM Nelson Chu wrote: >> >> The .insn directive can let users use their own instructions, or >> some new instruction, which haven't supported in the old binutils. >> For example, if users want to use sifive cache instruction, they >> cannot just write "cflush.d1.l1" in the assembly code, they should >> use ".insn i SYSTEM, 0, x0, x10, -0x40". But the .insn directive >> may not easy to use for some cases, and not so friendly to users. >> Therefore, I believe most of the users will use ".word 0xfc050073", >> to encode the instructions directly, rather than use .insn. But >> once we have supported the mapping symbols, the .word directives >> are marked as data, so disassembler won't dump them as instructions >> as usual. I have discussed this with Kito many times, we all think >> extend the .insn direcitve to support the hardcode encoding, is the >> easiest way to resolve the problem. Therefore, there are two more >> .insn formats are proposed as follows, >> >> (original) .insn , , , ... >> .insn , >> .insn >> >> The is string, and the and are constants. IMO we'd be way better off just having the actual instructions in tree than relying on users to know about the bits driectly. I'm still fine with .insn, as it's essentially necessary to make the mapping symbols work and it's better than what users are currently doing. >> ChangeLog: >> >> gas/ >> >> pr 27916 >> * config/tc-riscv.c (riscv_ip_hardcode): Similar to riscv_ip, >> but assembles an instruction according to the hardcode values >> of .insn directive. >> * testsuite/gas/riscv/insn-fail.d: New testcases. >> * testsuite/gas/riscv/insn-fail.l: Likewise. >> * testsuite/gas/riscv/insn-fail.s: Likewise. >> * testsuite/gas/riscv/insn.d: Updated. >> * testsuite/gas/riscv/insn.s: Likewise. >> --- >> gas/config/tc-riscv.c | 57 +++++++++++++++++++++++++++-- >> gas/testsuite/gas/riscv/insn-fail.d | 3 ++ >> gas/testsuite/gas/riscv/insn-fail.l | 7 ++++ >> gas/testsuite/gas/riscv/insn-fail.s | 6 +++ >> gas/testsuite/gas/riscv/insn.d | 6 +++ >> gas/testsuite/gas/riscv/insn.s | 7 ++++ >> 6 files changed, 83 insertions(+), 3 deletions(-) >> create mode 100644 gas/testsuite/gas/riscv/insn-fail.d >> create mode 100644 gas/testsuite/gas/riscv/insn-fail.l >> create mode 100644 gas/testsuite/gas/riscv/insn-fail.s >> >> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c >> index 55c49471825..e9be17f56d1 100644 >> --- a/gas/config/tc-riscv.c >> +++ b/gas/config/tc-riscv.c >> @@ -2922,6 +2922,50 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr, >> return error; >> } >> >> +/* Similar to riscv_ip, but assembles an instruction according to the >> + hardcode values of .insn directive. */ >> + >> +static const char * >> +riscv_ip_hardcode (char *str, >> + struct riscv_cl_insn *ip, >> + expressionS *imm_expr, >> + const char *error) >> +{ >> + struct riscv_opcode *insn; >> + insn_t values[2] = {0, 0}; >> + unsigned int num = 0; >> + >> + input_line_pointer = str; >> + do >> + { >> + expression (imm_expr); >> + if (imm_expr->X_op != O_constant) >> + { >> + /* The first value isn't constant, so it should be >> + .insn . Call riscv_ip to parse it. */ >> + if (num == 0) >> + return error; >> + return _("values must be constant"); >> + } >> + values[num++] = (insn_t) imm_expr->X_add_number; >> + } >> + while (*input_line_pointer++ == ',' && num < 2); >> + >> + input_line_pointer--; >> + if (*input_line_pointer != '\0') >> + return _("unrecognized values"); >> + >> + insn = XNEW (struct riscv_opcode); >> + insn->match = values[num - 1]; >> + create_insn (ip, insn); >> + unsigned int bytes = riscv_insn_length (insn->match); >> + if (values[num - 1] >> (8 * bytes) != 0 >> + || (num == 2 && values[0] != bytes)) >> + return _("value conflicts with instruction length"); >> + >> + return NULL; >> +} >> + >> void >> md_assemble (char *str) >> { >> @@ -3914,7 +3958,10 @@ s_riscv_leb128 (int sign) >> return s_leb128 (sign); >> } >> >> -/* Parse the .insn directive. */ >> +/* Parse the .insn directive. There are three formats, >> + Format 1: .insn , , ... >> + Format 2: .insn , >> + Format 3: .insn . */ >> >> static void >> s_riscv_insn (int x ATTRIBUTE_UNUSED) >> @@ -3935,11 +3982,15 @@ s_riscv_insn (int x ATTRIBUTE_UNUSED) >> >> const char *error = riscv_ip (str, &insn, &imm_expr, >> &imm_reloc, insn_type_hash); >> - >> if (error) >> { >> - as_bad ("%s `%s'", error, str); >> + char *save_in = input_line_pointer; >> + error = riscv_ip_hardcode (str, &insn, &imm_expr, error); >> + input_line_pointer = save_in; >> } >> + >> + if (error) >> + as_bad ("%s `%s'", error, str); >> else >> { >> gas_assert (insn.insn_mo->pinfo != INSN_MACRO); >> diff --git a/gas/testsuite/gas/riscv/insn-fail.d b/gas/testsuite/gas/riscv/insn-fail.d >> new file mode 100644 >> index 00000000000..3548e85415a >> --- /dev/null >> +++ b/gas/testsuite/gas/riscv/insn-fail.d >> @@ -0,0 +1,3 @@ >> +#as: >> +#source: insn-fail.s >> +#error_output: insn-fail.l >> diff --git a/gas/testsuite/gas/riscv/insn-fail.l b/gas/testsuite/gas/riscv/insn-fail.l >> new file mode 100644 >> index 00000000000..e47d106b39b >> --- /dev/null >> +++ b/gas/testsuite/gas/riscv/insn-fail.l >> @@ -0,0 +1,7 @@ >> +.*Assembler messages: >> +.*Error: unrecognized opcode `r,0x00000013' >> +.*Error: values must be constant `0x4,rs1' >> +.*Error: unrecognized values `0x4 0x5' >> +.*Error: unrecognized values `0x4,0x5,0x6' >> +.*Error: value conflicts with instruction length `0x4,0x0001' >> +.*Error: value conflicts with instruction length `0x2,0x00000013' >> diff --git a/gas/testsuite/gas/riscv/insn-fail.s b/gas/testsuite/gas/riscv/insn-fail.s >> new file mode 100644 >> index 00000000000..064211d985d >> --- /dev/null >> +++ b/gas/testsuite/gas/riscv/insn-fail.s >> @@ -0,0 +1,6 @@ >> + .insn r, 0x00000013 >> + .insn 0x4, rs1 >> + .insn 0x4 0x5 >> + .insn 0x4, 0x5, 0x6 >> + .insn 0x4, 0x0001 >> + .insn 0x2, 0x00000013 >> diff --git a/gas/testsuite/gas/riscv/insn.d b/gas/testsuite/gas/riscv/insn.d >> index 8cb3d64b1a5..4edacc63368 100644 >> --- a/gas/testsuite/gas/riscv/insn.d >> +++ b/gas/testsuite/gas/riscv/insn.d >> @@ -69,3 +69,9 @@ Disassembly of section .text: >> [^:]+:[ ]+00c58533[ ]+add[ ]+a0,a1,a2 >> [^:]+:[ ]+00c58533[ ]+add[ ]+a0,a1,a2 >> [^:]+:[ ]+00c58533[ ]+add[ ]+a0,a1,a2 >> +[^:]+:[ ]+0001[ ]+nop >> +[^:]+:[ ]+00000013[ ]+nop >> +[^:]+:[ ]+00000057[ ]+0x57 >> +[^:]+:[ ]+0001[ ]+nop >> +[^:]+:[ ]+00000013[ ]+nop >> +[^:]+:[ ]+00000057[ ]+0x57 >> diff --git a/gas/testsuite/gas/riscv/insn.s b/gas/testsuite/gas/riscv/insn.s >> index 937ad119ff2..84739056b1a 100644 >> --- a/gas/testsuite/gas/riscv/insn.s >> +++ b/gas/testsuite/gas/riscv/insn.s >> @@ -53,3 +53,10 @@ target: >> .insn r 0x33, 0, 0, fa0, a1, fa2 >> .insn r 0x33, 0, 0, a0, fa1, fa2 >> .insn r 0x33, 0, 0, fa0, fa1, fa2 >> + >> + .insn 0x0001 >> + .insn 0x00000013 >> + .insn 0x00000057 >> + .insn 0x2, 0x0001 >> + .insn 0x4, 0x00000013 >> + .insn 0x4, 0x00000057 >> -- >> 2.30.2 >> Reviewed-by: Palmer Dabbelt Thanks!