qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 08/21] target/riscv: adding accessors to the registers upp


From: Richard Henderson
Subject: Re: [PATCH v3 08/21] target/riscv: adding accessors to the registers upper part
Date: Wed, 20 Oct 2021 08:09:13 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0

On 10/19/21 2:47 AM, Frédéric Pétrot wrote:
+static TCGv get_gprh(DisasContext *ctx, int reg_num)
+{
+    if (reg_num == 0 || get_ol(ctx) < MXL_RV128) {
+        return ctx->zero;
+    }

So... why return anything for OL < 128?
Seems like that should be a bug.

+static void gen_set_gprh(DisasContext *ctx, int reg_num, TCGv t)
+{
+    if (reg_num != 0) {
+        if (get_ol(ctx) < MXL_RV128) {
+            tcg_gen_sari_tl(cpu_gprh[reg_num], cpu_gpr[reg_num], 63);
+        } else {
+            tcg_gen_mov_tl(cpu_gprh[reg_num], t);
+        }
+    }

Hmm... this implies that you must set the low part first, which could be easy to mis-use. Probably better to create a combined gen_set_gpr128 that takes both halves at once.

+    /* devilish temporary code so that the patch compiles */
+    if (get_xl_max(ctx) == MXL_RV128) {
+        (void)get_gprh(ctx, 6);
+        (void)dest_gprh(ctx, 6);
+        gen_set_gprh(ctx, 6, NULL);
+    }

I don't think it would be confusing to squash this patch into the next one, which adds the actual uses.


r~



reply via email to

[Prev in Thread] Current Thread [Next in Thread]