| Message ID | 20250910120916.1103023-1-jiawei@iscas.ac.cn |
|---|---|
| State | New |
| Headers |
Return-Path: <binutils-bounces~patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D788D3858D35 for <patchwork@sourceware.org>; Wed, 10 Sep 2025 12:10:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D788D3858D35 X-Original-To: binutils@sourceware.org Delivered-To: binutils@sourceware.org Received: from cstnet.cn (smtp84.cstnet.cn [159.226.251.84]) by sourceware.org (Postfix) with ESMTPS id 092FB3858D35 for <binutils@sourceware.org>; Wed, 10 Sep 2025 12:09:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 092FB3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=iscas.ac.cn Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=iscas.ac.cn ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 092FB3858D35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=159.226.251.84 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1757506194; cv=none; b=dYcHnmAFXiA7+zyjVs/CKu/d7rf/UUNXrS5NNKclYu0KPAHEAiBH+ErOCU4JZxpco5TMLYIJJC/pjrKjbq5D6dCOMlqnuqXmZT9/8iu9mt+y2clFOuJC9hcJATefFuTS9V7NOvE3lkMZGxtZeZ82GfMBWr2rGIPi8UwifwtmBLU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1757506194; c=relaxed/simple; bh=cGcZy7wX7s8i3pq/og5KZLc2KPy0udmIinVUCrybkhs=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=PorZrdxXZ5990OarR8FjLbjVaSnIOL6JarU0rl4faTweOV6+lDYZ3WAv+mZXAU4xj52+YjWE7jLLws77kOHiHr8O5NEDz6gJsTeat5ljIwku2UY6PAJAQbk5mohpseSi06YmphdBPnFatDnhSY0IcDdDfWnK8ZE5A4twfJHPgHA= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 092FB3858D35 Received: from localhost.localdomain (unknown [210.73.43.101]) by APP-05 (Coremail) with SMTP id zQCowAAXuhONasFo+pcZAg--.35496S2; Wed, 10 Sep 2025 20:09:50 +0800 (CST) From: Jiawei <jiawei@iscas.ac.cn> To: binutils@sourceware.org Cc: nelson@rivosinc.com, jbeulich@suse.com, kito.cheng@gmail.com, christoph.muellner@vrull.eu, research_trasio@irq.a4lg.com, Jiawei <jiawei@iscas.ac.cn> Subject: [PATCH] testsuites: Fix RISC-V tests with 'nop' instruction. Date: Wed, 10 Sep 2025 20:09:16 +0800 Message-ID: <20250910120916.1103023-1-jiawei@iscas.ac.cn> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: zQCowAAXuhONasFo+pcZAg--.35496S2 X-Coremail-Antispam: 1UD129KBjvJXoWxCw4fAr1xKr4rGF4Uur4fAFb_yoWrJw1fpr WkCF40kr95GFn7ArnrKr1UJa1xJws7Wr90934fAa4Ika1fGrW7t3Wktw47JF43tFWj9w1r CrsrZryFvF45tFUanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDU0xBIdaVrnRJUUUkl14x267AKxVW8JVW5JwAFc2x0x2IEx4CE42xK8VAvwI8IcIk0 rVWrJVCq3wAFIxvE14AKwVWUJVWUGwA2ocxC64kIII0Yj41l84x0c7CEw4AK67xGY2AK02 1l84ACjcxK6xIIjxv20xvE14v26r1I6r4UM28EF7xvwVC0I7IYx2IY6xkF7I0E14v26r4j 6F4UM28EF7xvwVC2z280aVAFwI0_Cr1j6rxdM28EF7xvwVC2z280aVCY1x0267AKxVW0oV Cq3wAS0I0E0xvYzxvE52x082IY62kv0487Mc02F40EFcxC0VAKzVAqx4xG6I80ewAv7VC0 I7IYx2IY67AKxVWUJVWUGwAv7VC2z280aVAFwI0_Gr0_Cr1lOx8S6xCaFVCjc4AY6r1j6r 4UM4x0Y48IcxkI7VAKI48JM4x0x7Aq67IIx4CEVc8vx2IErcIFxwCY1x0262kKe7AKxVWU AVWUtwCF04k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwC20s026c02F40E14 v26r1j6r18MI8I3I0E7480Y4vE14v26r106r1rMI8E67AF67kF1VAFwI0_JF0_Jw1lIxkG c2Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWUJVWUCwCI42IY6xIIjxv20xvEc7CjxVAFwI 0_Jr0_Gr1lIxAIcVCF04k26cxKx2IYs7xG6r1j6r1xMIIF0xvEx4A2jsIE14v26r1j6r4U MIIF0xvEx4A2jsIEc7CjxVAFwI0_Jr0_GrUvcSsGvfC2KfnxnUUI43ZEXa7VUjJ73PUUUU U== X-Originating-IP: [210.73.43.101] X-CM-SenderInfo: 5mld4v3l6l2u1dvotugofq/1tbiDAYBAGjBRmRrxgAAst X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_VALIDITY_RPBL_BLOCKED, RCVD_IN_VALIDITY_SAFE_BLOCKED, SPF_HELO_PASS, 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 X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Binutils mailing list <binutils.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/binutils>, <mailto:binutils-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/binutils/> List-Post: <mailto:binutils@sourceware.org> List-Help: <mailto:binutils-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/binutils>, <mailto:binutils-request@sourceware.org?subject=subscribe> Errors-To: binutils-bounces~patchwork=sourceware.org@sourceware.org |
| Series |
testsuites: Fix RISC-V tests with 'nop' instruction.
|
|
Commit Message
Jiawei
Sept. 10, 2025, 12:09 p.m. UTC
This patch forces tests containing 'nop'(0x13) instructions to specify the as parameter. The default as parameter is rv64gc will generation compressed instruction 'c.nop'(0x1) and broken the result as expected. Using 'rv64i' without 'C' extension to avoid this. gas/ChangeLog: * testsuite/gas/riscv/dis-partial-insn-byte.d: Limit as parameter. * testsuite/gas/riscv/dis-partial-insn-short.d: Ditto. * testsuite/gas/riscv/dis-partial-insn-word.d: Ditto. * testsuite/gas/riscv/no-relax-align.d: Ditto. * testsuite/gas/riscv/odd-padding.d: Ditto. * testsuite/gas/riscv/t_insns.d: Ditto. * testsuite/gas/riscv/tlsdesc.d: Ditto. --- gas/testsuite/gas/riscv/dis-partial-insn-byte.d | 2 +- gas/testsuite/gas/riscv/dis-partial-insn-short.d | 2 +- gas/testsuite/gas/riscv/dis-partial-insn-word.d | 2 +- gas/testsuite/gas/riscv/no-relax-align.d | 2 +- gas/testsuite/gas/riscv/odd-padding.d | 4 ++-- gas/testsuite/gas/riscv/t_insns.d | 2 +- gas/testsuite/gas/riscv/tlsdesc.d | 1 + 7 files changed, 8 insertions(+), 7 deletions(-)
Comments
On 10.09.2025 14:09, Jiawei wrote: > This patch forces tests containing 'nop'(0x13) instructions to specify the > as parameter. The default as parameter is rv64gc will generation compressed > instruction 'c.nop'(0x1) and broken the result as expected. I don't follow this explanation. There's no breakage right now, and you also don't alter any of the test expectations. Was the 2nd sentence maybe intended to start "If the default gas setting is rv64gc ..."? (I.e. if an override of DEFAULT_RISCV_ARCH_WITH_EXT was in effect.) If so, is NOP really the only thing we would need to be concerned of? Jan
On Wed, Sep 10, 2025 at 8:59 PM Jan Beulich <jbeulich@suse.com> wrote: > On 10.09.2025 14:09, Jiawei wrote: > > This patch forces tests containing 'nop'(0x13) instructions to specify > the > > as parameter. The default as parameter is rv64gc will generation > compressed > > instruction 'c.nop'(0x1) and broken the result as expected. > > I don't follow this explanation. There's no breakage right now, and you > also > don't alter any of the test expectations. Was the 2nd sentence maybe > intended > to start "If the default gas setting is rv64gc ..."? (I.e. if an override > of > DEFAULT_RISCV_ARCH_WITH_EXT was in effect.) Yeah I guess that's the problem, but the weird thing is I suppose the regressions of riscv-gnu-toolchain should test it, but I don't get any failures... I think it's fine to make test cases passed for specific DEFAULT_RISCV_ARCH_WITH_EXT build, but force them to rv64i means we won't test rv32 for those test cases, which I think original we also wants rv32 were tested. Does the .option norvc directives work for those testcase? It seems only the difference between nop and c.nop, and we don't want c.nop breaks the test cases. For the dis-partial-insn-byte, rv64i generates nop, and rv64gc generates c.nop, which breaks the "objdump --start-address 0 --stop-address 1 -d", the former shows .byte 0x13, but the latter shows .byte 0x01. > If so, is NOP really the only thing > we would need to be concerned of? Well... Yeah probably more need to be concerned. Hi Jiawei, if you have time, could you please help to check out why riscv-gnu-toolchain regressions don't cause these problems? I remember that when muti-lib is not built, the toolchains should be built by --with-arch and --with-isa-spec configure options. Thanks :-) Nelson
On 9/10/25 9:44 AM, Nelson Chu wrote: > Does the .option norvc directives work for those testcase? That should work. >> If so, is NOP really the only thing >> we would need to be concerned of? > > > Well... Yeah probably more need to be concerned. I had to use ".option norvc" for the libffi risc-v static trampoline support [1] so that the "jr t2" in the commit wouldn't be automatically converted to c.jr, so yeah we have to be concerned by more than just nop. I'll also note gas and LLVM's assembler don't always agree when to convert an instruction to the compressed form when it is possible. Maybe different defaults within the assemblers? Peter [1] https://github.com/libffi/libffi/pull/933/commits/0a0897a3ff6deb5fd2afc1b995a7aa515c60dd66
Hi Jan, Actually, the regression test will fail on these cases when you update your riscv-gnu-toolchain repo into the latest version(cfe9d78f8dc8dffc8ce371c46978b257ef2a5c35). I think you are right, it's not a problem with NOP, it's just a problem for no-compressed instruction test now be compressed. BR, Jiawei In 2025/9/10 20:59, Jan Beulich wrote: > On 10.09.2025 14:09, Jiawei wrote: >> This patch forces tests containing 'nop'(0x13) instructions to specify the >> as parameter. The default as parameter is rv64gc will generation compressed >> instruction 'c.nop'(0x1) and broken the result as expected. > I don't follow this explanation. There's no breakage right now, and you also > don't alter any of the test expectations. Was the 2nd sentence maybe intended > to start "If the default gas setting is rv64gc ..."? (I.e. if an override of > DEFAULT_RISCV_ARCH_WITH_EXT was in effect.) If so, is NOP really the only thing > we would need to be concerned of? > > Jan
In 2025/9/10 22:44, Nelson Chu wrote: > > > On Wed, Sep 10, 2025 at 8:59 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 10.09.2025 14:09, Jiawei wrote: > > This patch forces tests containing 'nop'(0x13) instructions to > specify the > > as parameter. The default as parameter is rv64gc will generation > compressed > > instruction 'c.nop'(0x1) and broken the result as expected. > > I don't follow this explanation. There's no breakage right now, > and you also > don't alter any of the test expectations. Was the 2nd sentence > maybe intended > to start "If the default gas setting is rv64gc ..."? (I.e. if an > override of > DEFAULT_RISCV_ARCH_WITH_EXT was in effect.) > > > Yeah I guess that's the problem, but the weird thing is I suppose the > regressions of riscv-gnu-toolchain should test it, but I don't get any > failures... I think it's fine to make test cases passed for specific > DEFAULT_RISCV_ARCH_WITH_EXT build, but force them to rv64i means we > won't test rv32 for those test cases, which I think original we also > wants rv32 were tested. Does the .option norvc directives work for > those testcase? It seems only the difference between nop and c.nop, > and we don't want c.nop breaks the test cases. > > For the dis-partial-insn-byte, rv64i generates nop, and rv64gc > generates c.nop, which breaks the "objdump --start-address 0 > --stop-address 1 -d", the former shows .byte 0x13, but the latter > shows .byte 0x01. Yes, that's the problem, before the riscv-gnu-toolchain update, --with-arch option will not be passed into binutils part, but currently it does, https://github.com/riscv-collab/riscv-gnu-toolchain/commit/1faa77ffe8daec705aa1ef266e944c554d838881 so some test will show compressed result and come into fail. Thanks for your hint, the current revision is indeed one-sided. I think maybe add .option norvc in assemble file is a better choice. BR, Jiawei > If so, is NOP really the only thing > we would need to be concerned of? > > > Well... Yeah probably more need to be concerned. Hi Jiawei, if you > have time, could you please help to check out why riscv-gnu-toolchain > regressions don't cause these problems? I remember that when muti-lib > is not built, the toolchains should be built by --with-arch and > --with-isa-spec configure options. Thanks :-) > > Nelson
In 2025/9/10 23:39, Peter Bergner wrote: > On 9/10/25 9:44 AM, Nelson Chu wrote: >> Does the .option norvc directives work for those testcase? > That should work. > > > >>> If so, is NOP really the only thing >>> we would need to be concerned of? >> >> Well... Yeah probably more need to be concerned. > I had to use ".option norvc" for the libffi risc-v static trampoline > support [1] so that the "jr t2" in the commit wouldn't be automatically > converted to c.jr, so yeah we have to be concerned by more than just > nop. Thanks for your suggestions, I will try this way. BR, Jiawei > > I'll also note gas and LLVM's assembler don't always agree when to convert > an instruction to the compressed form when it is possible. Maybe different > defaults within the assemblers? > > > Peter > > > [1] https://github.com/libffi/libffi/pull/933/commits/0a0897a3ff6deb5fd2afc1b995a7aa515c60dd66
On Thu, Sep 11, 2025 at 9:52 AM Jiawei <jiawei@iscas.ac.cn> wrote: > In 2025/9/10 22:44, Nelson Chu wrote: > > On Wed, Sep 10, 2025 at 8:59 PM Jan Beulich <jbeulich@suse.com> wrote: > >> On 10.09.2025 14:09, Jiawei wrote: >> > This patch forces tests containing 'nop'(0x13) instructions to specify >> the >> > as parameter. The default as parameter is rv64gc will generation >> compressed >> > instruction 'c.nop'(0x1) and broken the result as expected. >> >> I don't follow this explanation. There's no breakage right now, and you >> also >> don't alter any of the test expectations. Was the 2nd sentence maybe >> intended >> to start "If the default gas setting is rv64gc ..."? (I.e. if an override >> of >> DEFAULT_RISCV_ARCH_WITH_EXT was in effect.) > > > Yeah I guess that's the problem, but the weird thing is I suppose the > regressions of riscv-gnu-toolchain should test it, but I don't get any > failures... I think it's fine to make test cases passed for specific > DEFAULT_RISCV_ARCH_WITH_EXT build, but force them to rv64i means we won't > test rv32 for those test cases, which I think original we also wants rv32 > were tested. Does the .option norvc directives work for those testcase? > It seems only the difference between nop and c.nop, and we don't want c.nop > breaks the test cases. > > For the dis-partial-insn-byte, rv64i generates nop, and rv64gc generates > c.nop, which breaks the "objdump --start-address 0 --stop-address 1 -d", > the former shows .byte 0x13, but the latter shows .byte 0x01. > > Yes, that's the problem, before the riscv-gnu-toolchain update, > --with-arch option will not be passed into binutils part, but currently it > does, > > > https://github.com/riscv-collab/riscv-gnu-toolchain/commit/1faa77ffe8daec705aa1ef266e944c554d838881 > > so some test will show compressed result and come into fail. > Thanks! So we should fix these failures in binutils test cases. > Thanks for your hint, the current revision is indeed one-sided. I think > maybe add .option norvc in assemble file is a better choice. > Cool. As the above discussions with Jan and Peter, I think it worth to try .option norvc first. Nelson
On Wed, Sep 10, 2025 at 11:39 PM Peter Bergner <bergner@tenstorrent.com> wrote: > On 9/10/25 9:44 AM, Nelson Chu wrote: > > Does the .option norvc directives work for those testcase? > > That should work. > > > > >> If so, is NOP really the only thing > >> we would need to be concerned of? > > > > > > Well... Yeah probably more need to be concerned. > > I had to use ".option norvc" for the libffi risc-v static trampoline > support [1] so that the "jr t2" in the commit wouldn't be automatically > converted to c.jr, so yeah we have to be concerned by more than just > nop. > > I'll also note gas and LLVM's assembler don't always agree when to convert > an instruction to the compressed form when it is possible. Maybe different > defaults within the assemblers? > I remember there is a new directive .option exact/noexact under community discussion, it may be what you want ;) Nelson
diff --git a/gas/testsuite/gas/riscv/dis-partial-insn-byte.d b/gas/testsuite/gas/riscv/dis-partial-insn-byte.d index f8c149ca881..69252a671c6 100644 --- a/gas/testsuite/gas/riscv/dis-partial-insn-byte.d +++ b/gas/testsuite/gas/riscv/dis-partial-insn-byte.d @@ -1,4 +1,4 @@ -#as: +#as: -march=rv64i #source: dis-partial-insn.s #objdump: --start-address 0 --stop-address 1 -d diff --git a/gas/testsuite/gas/riscv/dis-partial-insn-short.d b/gas/testsuite/gas/riscv/dis-partial-insn-short.d index 4a7d6d98b82..cddf235771b 100644 --- a/gas/testsuite/gas/riscv/dis-partial-insn-short.d +++ b/gas/testsuite/gas/riscv/dis-partial-insn-short.d @@ -1,4 +1,4 @@ -#as: +#as: -march=rv64i #source: dis-partial-insn.s #objdump: --start-address 0 --stop-address 2 -d diff --git a/gas/testsuite/gas/riscv/dis-partial-insn-word.d b/gas/testsuite/gas/riscv/dis-partial-insn-word.d index af30c5be3d5..84d1652f278 100644 --- a/gas/testsuite/gas/riscv/dis-partial-insn-word.d +++ b/gas/testsuite/gas/riscv/dis-partial-insn-word.d @@ -1,4 +1,4 @@ -#as: +#as: -march=rv64i #source: dis-partial-insn.s #objdump: --start-address 0 --stop-address 3 -d diff --git a/gas/testsuite/gas/riscv/no-relax-align.d b/gas/testsuite/gas/riscv/no-relax-align.d index ced07d3c830..431a962d23f 100644 --- a/gas/testsuite/gas/riscv/no-relax-align.d +++ b/gas/testsuite/gas/riscv/no-relax-align.d @@ -1,4 +1,4 @@ -#as: +#as: -march=rv64i #objdump: -dr .*:[ ]+file format .* diff --git a/gas/testsuite/gas/riscv/odd-padding.d b/gas/testsuite/gas/riscv/odd-padding.d index b445a74f346..6e712b5cf65 100644 --- a/gas/testsuite/gas/riscv/odd-padding.d +++ b/gas/testsuite/gas/riscv/odd-padding.d @@ -1,5 +1,5 @@ -#as: -mrelax -#as: -mno-relax +#as: -march=rv64i -mrelax +#as: -march=rv64i -mno-relax #objdump: -d .*:[ ]+file format .* diff --git a/gas/testsuite/gas/riscv/t_insns.d b/gas/testsuite/gas/riscv/t_insns.d index 720f0db2930..0988a3d8f64 100644 --- a/gas/testsuite/gas/riscv/t_insns.d +++ b/gas/testsuite/gas/riscv/t_insns.d @@ -1,4 +1,4 @@ -#as: +#as: -march=rv64i #objdump: -dr .*:[ ]+file format .* diff --git a/gas/testsuite/gas/riscv/tlsdesc.d b/gas/testsuite/gas/riscv/tlsdesc.d index 5cd26ac3280..2ff630572f9 100644 --- a/gas/testsuite/gas/riscv/tlsdesc.d +++ b/gas/testsuite/gas/riscv/tlsdesc.d @@ -1,3 +1,4 @@ +#as: -march=rv64i #source: tlsdesc.s #objdump: -dr