[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-riscv] [Qemu-devel] [PATCH 0/2] target/riscv: Bugfixes found i
From: |
Palmer Dabbelt |
Subject: |
Re: [Qemu-riscv] [Qemu-devel] [PATCH 0/2] target/riscv: Bugfixes found in decodetree conversion |
Date: |
Thu, 08 Nov 2018 11:12:00 -0800 (PST) |
On Thu, 08 Nov 2018 09:29:26 PST (-0800), Bastian Koppelmann wrote:
On 11/8/18 4:53 PM, Richard Henderson wrote:
On 11/8/18 1:06 PM, Bastian Koppelmann wrote:
while going through the reviews of the riscv-decodetree patches, two bugs came
up that I fix here. There is one more problem [1] mentioned by Richard but
I don't have the time to investigate it further.
[1] https://patchwork.kernel.org/patch/10650293/
That one's not a bug, but an optimization.
The other bug mentioned is shrw and shaw not sign-extending the result.
That was a bug I introduced during the conversion to decodetree.
My understand of that patch feedback is that there are two issues:
* We don't take advantage of the ordering bits on fences, which could allow us
to emit less conservative fences. This would presumably increase
performance.
* We treat fences as NOPs under "#ifndef CONFIG_USER_ONLY", which is a bug.
Am I wrong about that second one? I think the fix should look something like
this, which I haven't even tried to compile
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 18d7b6d1471d..624d1c679a84 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1766,7 +1766,6 @@ static void decode_RV32_64G(CPURISCVState *env,
DisasContext *ctx)
GET_RM(ctx->opcode));
break;
case OPC_RISC_FENCE:
-#ifndef CONFIG_USER_ONLY
if (ctx->opcode & 0x1000) {
/* FENCE_I is a no-op in QEMU,
* however we need to end the translation block */
@@ -1777,7 +1776,6 @@ static void decode_RV32_64G(CPURISCVState *env,
DisasContext *ctx)
/* FENCE is a full memory barrier. */
tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
}
-#endif
break;
case OPC_RISC_SYSTEM:
gen_system(env, ctx, MASK_OP_SYSTEM(ctx->opcode), rd, rs1,