Re: [PATCH v3 11/21] target/riscv: support for 128-bit bitwise instructions

[Richard Henderson]

On 10/19/21 2:48 AM, Frédéric Pétrot wrote:
The 128-bit bitwise instructions do not need any function prototype change
as the functions can be applied independently on the lower and upper part of
the registers.
Signed-off-by: Frédéric Pétrot <frederic.pet...@univ-grenoble-alpes.fr>
Co-authored-by: Fabien Portas <fabien.por...@grenoble-inp.org>
---
  target/riscv/translate.c | 22 ++++++++++++++++++++++
  1 file changed, 22 insertions(+)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index e8f08f921e..71982f6284 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -429,6 +429,17 @@ static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a, 
DisasExtend ext,
  
      gen_set_gpr(ctx, a->rd, dest);
  
+    if (get_xl_max(ctx) == MXL_RV128) {
+        if (get_ol(ctx) ==  MXL_RV128) {
+            uint64_t immh = -(a->imm < 0);
+            src1 = get_gprh(ctx, a->rs1);
+            dest = dest_gprh(ctx, a->rd);
+
+            func(dest, src1, immh);
+        }
+        gen_set_gprh(ctx, a->rd, dest);
+    }

If ol < RV128, you're storing the low dest into the gprh, which is wrong.  It should be 
the sign-extension of the low part.  But that should happen for all writes.

Earlier, I suggested gen_set_gpr128 instead of gen_set_gprh.
I think this should be written

    if (get_xl(ctx) == MXL_RV128) {
        TCGv src1h = get_gprh(ctx, a->rs1);
        TCGv desth = dest_gprh(ctx, a->rd);

        func(dest, src1h, -(a->imm < 0));
        gen_set_gpr128(ctx, a->rd, dest, desth);
    } else {
        gen_set_gpr(ctx, a->rd, dest);
    }

Where gen_set_gpr will handle the sign-extension to 128-bits.


@@ -443,6 +454,17 @@ static bool gen_logic(DisasContext *ctx, arg_r *a, 
DisasExtend ext,
  
      gen_set_gpr(ctx, a->rd, dest);
  
+    if (get_xl_max(ctx) == MXL_RV128) {
+        if (get_ol(ctx) ==  MXL_RV128) {
+            dest = dest_gprh(ctx, a->rd);
+            src1 = get_gprh(ctx, a->rs1);
+            src2 = get_gprh(ctx, a->rs2);
+
+            func(dest, src1, src2);
+        }
+        gen_set_gprh(ctx, a->rd, dest);
+    }

Similarly.


r~