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

[Richard Henderson]

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~