i#3544 RV64: Treat atomic instructions as load/store instructions (#6528

)

Atomic instructions from the A extension like LR/SC and AMO* access
memory so they should be treated as load/store. In addition, AMO*
instructions perform read-modify-write operations, they should have both
a source and a destination operands to comply with DR's IR convention.
We make the destination an implicit operand as they are identical to the
source.

Related PR: #6506
Issue: #3544

core/arch/riscv64/mangle.c

@@ -724,19 +724,18 @@ mangle_exclusive_load(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
     src0 = instr_get_src(instr, 0);
     opcode = instr_get_opcode(instr) == OP_lr_d ? OP_ld : OP_lw;
     opsz = opcode == OP_ld ? OPSZ_8 : OPSZ_4;
-    ASSERT(opnd_is_reg(dst) && opnd_is_reg(src0));
+    ASSERT(opnd_is_reg(dst) && opnd_is_base_disp(src0));
     if (opnd_get_reg(dst) == dr_reg_stolen) {
         opnd_replace_reg(&dst, dr_reg_stolen, scratch_reg2);
     }
-    if (opnd_get_reg(src0) == dr_reg_stolen) {
+    if (opnd_get_base(src0) == dr_reg_stolen) {
         opnd_replace_reg(&src0, dr_reg_stolen, scratch_reg2);
     }
     instr_reset(dcontext, instr);
     instr_set_opcode(instr, opcode);
     instr_set_num_opnds(dcontext, instr, 1, 1);
     instr_set_dst(instr, 0, dst);
-    instr_set_src(instr, 0,
-                  opnd_create_base_disp(opnd_get_reg(src0), DR_REG_NULL, 0, 0, opsz));
+    instr_set_src(instr, 0, src0);
     instr_set_translation(instr, instrlist_get_translation_target(ilist));
 
     /* Keep the acquire semantics if needed. */
@@ -751,7 +750,7 @@ mangle_exclusive_load(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
 
     /* Save address, value and size to TLS slot. */
     PRE(ilist, next_instr,
-        instr_create_save_to_tls(dcontext, opnd_get_reg(src0), TLS_LRSC_ADDR_SLOT));
+        instr_create_save_to_tls(dcontext, opnd_get_base(src0), TLS_LRSC_ADDR_SLOT));
     PRE(ilist, next_instr,
         instr_create_save_to_tls(dcontext, opnd_get_reg(dst), TLS_LRSC_VALUE_SLOT));
     PRE(ilist, next_instr,
@@ -779,19 +778,20 @@ mangle_exclusive_store(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
     reg_id_t scratch_reg1, scratch_reg2;
     ushort slot1, slot2;
     int opcode;
-    opnd_t dst, src0;
+    opnd_t dst0, dst1;
     opnd_size_t opsz;
     instr_t *fail = INSTR_CREATE_label(dcontext), *final = INSTR_CREATE_label(dcontext),
             *loop = INSTR_CREATE_label(dcontext);
     ASSERT(instr_is_exclusive_store(instr));
-    ASSERT(instr_num_dsts(instr) == 1 && instr_num_srcs(instr) == 3);
+    ASSERT(instr_num_dsts(instr) == 2 && instr_num_srcs(instr) == 2);
 
     /* TODO i#3544: Not implemented.  */
     ASSERT_NOT_IMPLEMENTED(!instr_uses_reg(instr, dr_reg_stolen));
     ASSERT_NOT_IMPLEMENTED(!instr_uses_reg(instr, DR_REG_TP));
 
-    dst = instr_get_dst(instr, 0);
-    src0 = instr_get_src(instr, 0);
+    dst0 = instr_get_dst(instr, 0);
+    dst1 = instr_get_dst(instr, 1);
+    ASSERT(opnd_is_base_disp(dst1));
     opsz = instr_get_opcode(instr) == OP_sc_d ? OPSZ_8 : OPSZ_4;
     scratch_reg1 = pick_scratch_reg(dcontext, instr, DR_REG_NULL, &slot1);
     scratch_reg2 = pick_scratch_reg(dcontext, instr, scratch_reg1, &slot2);
@@ -800,12 +800,12 @@ mangle_exclusive_store(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
     PRE(ilist, instr, instr_create_save_to_tls(dcontext, scratch_reg1, slot1));
     PRE(ilist, instr, instr_create_save_to_tls(dcontext, scratch_reg2, slot2));
 
-    /* Restore address saved by exclusive load and check if it's equal to src0. */
+    /* Restore address saved by exclusive load and check if it's equal to dst1. */
     PRE(ilist, instr,
         instr_create_restore_from_tls(dcontext, scratch_reg1, TLS_LRSC_ADDR_SLOT));
     PRE(ilist, instr,
         INSTR_CREATE_bne(dcontext, opnd_create_instr(fail), opnd_create_reg(scratch_reg1),
-                         src0));
+                         opnd_create_reg(opnd_get_base(dst1))));
 
     /* Restore size saved by exclusive load and check if it's equal to current size. */
     PRE(ilist, instr,
@@ -826,7 +826,7 @@ mangle_exclusive_store(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
      */
     opcode = instr_get_opcode(instr) == OP_sc_d ? OP_lr_d : OP_lr_w;
     PRE(ilist, instr,
-        instr_create_1dst_2src(dcontext, opcode, opnd_create_reg(scratch_reg2), src0,
+        instr_create_1dst_2src(dcontext, opcode, opnd_create_reg(scratch_reg2), dst1,
                                opnd_create_immed_int(0b11, OPSZ_2b)));
     PRE(ilist, instr,
         INSTR_CREATE_bne(dcontext, opnd_create_instr(final),
@@ -835,7 +835,7 @@ mangle_exclusive_store(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
     /* instr is here. */
 
     PRE(ilist, next_instr,
-        INSTR_CREATE_bne(dcontext, opnd_create_instr(loop), dst,
+        INSTR_CREATE_bne(dcontext, opnd_create_instr(loop), dst0,
                          opnd_create_reg(DR_REG_ZERO)));
     /* End of the LR/SC sequence. */
 
@@ -844,7 +844,7 @@ mangle_exclusive_store(dcontext_t *dcontext, instrlist_t *ilist, instr_t *instr,
     /* Write a non-zero value to dst on fail. */
     PRE(ilist, next_instr, fail);
     PRE(ilist, next_instr,
-        XINST_CREATE_load_int(dcontext, dst, opnd_create_immed_int(1, OPSZ_12b)));
+        XINST_CREATE_load_int(dcontext, dst0, opnd_create_immed_int(1, OPSZ_12b)));
 
     PRE(ilist, next_instr, final);
 

core/ir/riscv64/codec.c

@@ -90,8 +90,8 @@ typedef bool (*opnd_dec_func_t)(dcontext_t *dc, uint32_t inst, int op_sz, byte *
  * Helper functions.
  */
 
-#define INFO_NDST(opcode) GET_FIELD((opcode), 31, 31);
-#define INFO_NSRC(opcode) GET_FIELD((opcode), 30, 28);
+#define INFO_NDST(opcode) GET_FIELD((opcode), 31, 30);
+#define INFO_NSRC(opcode) GET_FIELD((opcode), 29, 27);
 
 /*
  * End of helper functions.
@@ -1049,7 +1049,9 @@ decode_v_l_rs1_disp_opnd(dcontext_t *dc, uint32_t inst, int op_sz, byte *pc,
                          byte *orig_pc, int idx, instr_t *out)
 {
     reg_t reg = DR_REG_X0 + GET_FIELD(inst, 19, 15);
-    int32_t imm = SIGN_EXTEND(GET_FIELD(inst, 31, 20), 12);
+    /* Immediate part of LR.W/D is always 0. */
+    int32_t imm =
+        GET_FIELD(inst, 6, 0) == 0b0101111 ? 0 : SIGN_EXTEND(GET_FIELD(inst, 31, 20), 12);
     opnd_t opnd = opnd_add_flags(opnd_create_base_disp(reg, DR_REG_NULL, 0, imm, op_sz),
                                  DR_OPND_IMM_PRINT_DECIMAL);
     instr_set_src(out, idx, opnd);
@@ -1072,7 +1074,10 @@ decode_v_s_rs1_disp_opnd(dcontext_t *dc, uint32_t inst, int op_sz, byte *pc,
                          byte *orig_pc, int idx, instr_t *out)
 {
     reg_t reg = DR_REG_X0 + GET_FIELD(inst, 19, 15);
-    int32_t imm = (GET_FIELD(inst, 31, 25) << 5) | GET_FIELD(inst, 11, 7);
+    /* Immediate part of SC.W/D is always 0. */
+    int32_t imm = GET_FIELD(inst, 6, 0) == 0b0101111
+        ? 0
+        : (GET_FIELD(inst, 31, 25) << 5) | GET_FIELD(inst, 11, 7);
     imm = SIGN_EXTEND(imm, 12);
     opnd_t opnd = opnd_add_flags(opnd_create_base_disp(reg, DR_REG_NULL, 0, imm, op_sz),
                                  DR_OPND_IMM_PRINT_DECIMAL);
@@ -1259,6 +1264,7 @@ opnd_dec_func_t opnd_decoders[] = {
     [RISCV64_FLD_IIMM_0] = decode_iimm_0_opnd,
     [RISCV64_FLD_ICRS1] = decode_icrs1_opnd,
     [RISCV64_FLD_ICRS1__] = decode_icrs1___opnd,
+    [RISCV64_FLD_I_S_RS1_DISP] = decode_v_s_rs1_disp_opnd,
 };
 
 /* Decode RVC quadrant 0.
@@ -1515,11 +1521,20 @@ decode_common(dcontext_t *dcontext, byte *pc, byte *orig_pc, instr_t *instr)
     instr_set_opcode(instr, info->info.type);
     instr_set_num_opnds(dcontext, instr, ndst, nsrc);
 
-    CLIENT_ASSERT(info->info.dst1_type < RISCV64_FLD_CNT, "Invalid dst1_type.");
-    if (ndst > 0 &&
-        !opnd_decoders[info->info.dst1_type](dcontext, inst, info->info.dst1_size, pc,
-                                             orig_pc, 0, instr))
-        goto decode_failure;
+    switch (ndst) {
+    case 2:
+        CLIENT_ASSERT(info->info.dst2_type < RISCV64_FLD_CNT, "Invalid dst2_type.");
+        if (!opnd_decoders[info->info.dst2_type](dcontext, inst, info->info.dst2_size, pc,
+                                                 orig_pc, 1, instr))
+            goto decode_failure;
+    case 1:
+        CLIENT_ASSERT(info->info.dst1_type < RISCV64_FLD_CNT, "Invalid dst1_type.");
+        if (!opnd_decoders[info->info.dst1_type](dcontext, inst, info->info.dst1_size, pc,
+                                                 orig_pc, 0, instr))
+            goto decode_failure;
+    case 0: break;
+    default: ASSERT_NOT_REACHED();
+    }
     switch (nsrc) {
     case 4:
         CLIENT_ASSERT(info->info.dst2_type < RISCV64_FLD_CNT, "Invalid dst2_type.");
@@ -2474,8 +2489,10 @@ encode_v_l_rs1_disp_opnd(instr_t *instr, byte *pc, int idx, uint32_t *out,
     opnd_t opnd = instr_get_src(instr, idx);
     uint32_t reg = opnd_get_base(opnd) - DR_REG_X0;
     *out |= SET_FIELD(reg, 19, 15);
-    int32_t imm = opnd_get_disp(opnd);
-    *out |= SET_FIELD(imm, 31, 20);
+    if (instr->opcode != OP_lr_w && instr->opcode != OP_lr_d) {
+        int32_t imm = opnd_get_disp(opnd);
+        *out |= SET_FIELD(imm, 31, 20);
+    }
     return true;
 }
 
@@ -2497,8 +2514,10 @@ encode_v_s_rs1_disp_opnd(instr_t *instr, byte *pc, int idx, uint32_t *out,
     opnd_t opnd = instr_get_dst(instr, idx);
     uint32_t reg = opnd_get_base(opnd) - DR_REG_X0;
     *out |= SET_FIELD(reg, 19, 15);
-    int32_t imm = opnd_get_disp(opnd);
-    *out |= SET_FIELD(imm, 11, 7) | SET_FIELD(imm >> 5, 31, 25);
+    if (instr->opcode != OP_sc_w && instr->opcode != OP_sc_d) {
+        int32_t imm = opnd_get_disp(opnd);
+        *out |= SET_FIELD(imm, 11, 7) | SET_FIELD(imm >> 5, 31, 25);
+    }
     return true;
 }
 
@@ -2566,6 +2585,7 @@ opnd_enc_func_t opnd_encoders[] = {
     [RISCV64_FLD_IIMM_0] = encode_implicit_opnd,
     [RISCV64_FLD_ICRS1] = encode_implicit_opnd,
     [RISCV64_FLD_ICRS1__] = encode_implicit_opnd,
+    [RISCV64_FLD_I_S_RS1_DISP] = encode_implicit_opnd,
 };
 
 uint
@@ -2582,9 +2602,18 @@ encode_common(byte *pc, instr_t *instr, decode_info_t *di)
     CLIENT_ASSERT(ndst >= 0 || ndst <= 1, "Invalid number of destination operands.");
     CLIENT_ASSERT(nsrc >= 0 || nsrc <= 4, "Invalid number of source operands.");
 
-    CLIENT_ASSERT(info->info.dst1_type < RISCV64_FLD_CNT, "Invalid dst1_type.");
-    if (ndst > 0 && !opnd_encoders[info->info.dst1_type](instr, pc, 0, &inst, di))
-        goto encode_failure;
+    switch (ndst) {
+    case 2:
+        CLIENT_ASSERT(info->info.dst2_type < RISCV64_FLD_CNT, "Invalid dst2_type.");
+        if (!opnd_encoders[info->info.dst2_type](instr, pc, 1, &inst, di))
+            goto encode_failure;
+    case 1:
+        CLIENT_ASSERT(info->info.dst1_type < RISCV64_FLD_CNT, "Invalid dst1_type.");
+        if (!opnd_encoders[info->info.dst1_type](instr, pc, 0, &inst, di))
+            goto encode_failure;
+    case 0: break;
+    default: ASSERT_NOT_REACHED();
+    }
     switch (nsrc) {
     case 4:
         CLIENT_ASSERT(info->info.dst2_type < RISCV64_FLD_CNT, "Invalid dst2_type.");

core/ir/riscv64/codec.h

@@ -255,6 +255,7 @@ typedef enum {
     RISCV64_FLD_IIMM_0,
     RISCV64_FLD_ICRS1,
     RISCV64_FLD_ICRS1__,
+    RISCV64_FLD_I_S_RS1_DISP,
     RISCV64_FLD_CNT, /* Keep this last */
 } riscv64_fld_t;
 

core/ir/riscv64/codec.py

@@ -596,7 +596,13 @@ def __new__(cls, value: int, arg_name: str, is_dest: bool, is_implicit: bool,
                         'ld': 'OPSZ_8', 'lbu': 'OPSZ_1', 'lhu': 'OPSZ_2', 'lwu': 'OPSZ_4',
                         'sb': 'OPSZ_1', 'sh': 'OPSZ_2', 'sw': 'OPSZ_4', 'sd': 'OPSZ_8',
                         'flw': 'OPSZ_4', 'fld': 'OPSZ_8', 'fsw': 'OPSZ_4', 'fsd': 'OPSZ_8',
-                        'flq': 'OPSZ_16', 'fsq': 'OPSZ_16'
+                        'flq': 'OPSZ_16', 'fsq': 'OPSZ_16', 'lr.w': 'OPSZ_4', 'lr.d': 'OPSZ_8',
+                        'amoswap.w': 'OPSZ_4', 'amoadd.w': 'OPSZ_4', 'amoxor.w': 'OPSZ_4',
+                        'amoand.w': 'OPSZ_4', 'amoor.w': 'OPSZ_4', 'amomin.w': 'OPSZ_4',
+                        'amomax.w': 'OPSZ_4', 'amominu.w': 'OPSZ_4', 'amomaxu.w': 'OPSZ_4',
+                        'amoswap.d': 'OPSZ_8', 'amoadd.d': 'OPSZ_8', 'amoxor.d': 'OPSZ_8',
+                        'amoand.d': 'OPSZ_8', 'amoor.d': 'OPSZ_8', 'amomin.d': 'OPSZ_8',
+                        'amomax.d': 'OPSZ_8', 'amominu.d': 'OPSZ_8', 'amomaxu.d': 'OPSZ_8',
                     },
                     'imm(rs1)',
                     'The register-relative memory source location (reg+imm).'
@@ -611,7 +617,7 @@ def __new__(cls, value: int, arg_name: str, is_dest: bool, is_implicit: bool,
                         'ld': 'OPSZ_8', 'lbu': 'OPSZ_1', 'lhu': 'OPSZ_2', 'lwu': 'OPSZ_4',
                         'sb': 'OPSZ_1', 'sh': 'OPSZ_2', 'sw': 'OPSZ_4', 'sd': 'OPSZ_8',
                         'flw': 'OPSZ_4', 'fld': 'OPSZ_8', 'fsw': 'OPSZ_4', 'fsd': 'OPSZ_8',
-                        'flq': 'OPSZ_16', 'fsq': 'OPSZ_16'
+                        'flq': 'OPSZ_16', 'fsq': 'OPSZ_16', 'sc.w': 'OPSZ_4', 'sc.d': 'OPSZ_8'
                     },
                     'imm(rs1)',
                     'The register-relative memory target location (reg+imm).'
@@ -697,6 +703,22 @@ def __new__(cls, value: int, arg_name: str, is_dest: bool, is_implicit: bool,
                'rs1',
                'Implicit rs1, same as CRD__.',
                )
+    I_S_RS1_DISP = (63,
+                    'Mem',
+                    True,
+                    True,
+                    True,
+                    {
+                        'amoswap.w': 'OPSZ_4', 'amoadd.w': 'OPSZ_4', 'amoxor.w': 'OPSZ_4',
+                        'amoand.w': 'OPSZ_4', 'amoor.w': 'OPSZ_4', 'amomin.w': 'OPSZ_4',
+                        'amomax.w': 'OPSZ_4', 'amominu.w': 'OPSZ_4', 'amomaxu.w': 'OPSZ_4',
+                        'amoswap.d': 'OPSZ_8', 'amoadd.d': 'OPSZ_8', 'amoxor.d': 'OPSZ_8',
+                        'amoand.d': 'OPSZ_8', 'amoor.d': 'OPSZ_8', 'amomin.d': 'OPSZ_8',
+                        'amomax.d': 'OPSZ_8', 'amominu.d': 'OPSZ_8', 'amomaxu.d': 'OPSZ_8',
+                    },
+                    'imm(rs1)',
+                    'The register-relative memory target location (reg+imm).'
+                    )
 
     def __str__(self) -> str:
         return self.name.lower().replace("fp", "(fp)")
@@ -830,6 +852,8 @@ def __fixup_compressed_inst(self, inst: Instruction):
 
     def __fixup_uncompressed_inst(self, inst: Instruction):
         opc = (inst.match & inst.mask) & 0x7F
+        funct3 = ((inst.match & inst.mask) >> 12) & 0x7
+        rs3 = ((inst.match & inst.mask) >> 27) & 0x1f
         if opc in [0b0000011, 0b0000111]:  # LOAD instructions
             dbg(f'fixup: {inst.name} {[f.name for f in inst.flds]}')
             inst.flds[0] = Field.V_L_RS1_DISP
@@ -840,6 +864,19 @@ def __fixup_uncompressed_inst(self, inst: Instruction):
             inst.flds[2] = Field.V_S_RS1_DISP
             inst.flds.pop(0)
             dbg(f'    -> {" " * len(inst.name)} {[f.name for f in inst.flds]}')
+        elif opc == 0b0101111 and (funct3 == 0b010 or funct3 == 0b011):
+            if rs3 == 0x2: # LR.W/D instructions
+                dbg(f'fixup: {inst.name} {[f.name for f in inst.flds]}')
+                inst.flds[1] = Field.V_L_RS1_DISP
+                dbg(f'    -> {" " * len(inst.name)} {[f.name for f in inst.flds]}')
+            elif rs3 == 0x3: # SC.W/D instructions
+                dbg(f'fixup: {inst.name} {[f.name for f in inst.flds]}')
+                inst.flds[2] = Field.V_S_RS1_DISP
+                dbg(f'    -> {" " * len(inst.name)} {[f.name for f in inst.flds]}')
+            else: # AMO instructions
+                dbg(f'fixup: {inst.name} {[f.name for f in inst.flds]}')
+                inst.flds[2] = Field.V_L_RS1_DISP
+                inst.flds.append(Field.I_S_RS1_DISP)
         elif inst.mask == 0x1f07fff and inst.match in [0x6013, 0x106013, 0x306013]:
             # prefetch.[irw] instructions
             dbg(f'fixup: {inst.name} {[f.name for f in inst.flds]}')
@@ -1211,10 +1248,12 @@ def generate_instr_info_trie(self, out_file, op_offset) -> bool:
                 asm_args += ' '
                 asm_args += ', '.join([f.asm_name() for f in flds])
                 isrc = 1
+                idst = 0
                 for f in flds:
                     if f.is_dest:
-                        oidx = 0
+                        oidx = idst
                         ndst += 1
+                        idst = 4
                     else:
                         oidx = isrc
                         isrc += 1
@@ -1225,7 +1264,7 @@ def generate_instr_info_trie(self, out_file, op_offset) -> bool:
             instr_infos.append(f'''[OP_{i.formatted_name()}] = {{ /* {i.name}{asm_args} */
     .info = {{
         .type = OP_{i.formatted_name()},
-        .opcode = 0x{(ndst << 31) | (nsrc << 28):08x}, /* {ndst} dst, {nsrc} src */
+        .opcode = 0x{(ndst << 30) | (nsrc << 27):08x}, /* {ndst} dst, {nsrc} src */
         .name = "{i.name}",{''.join(opnds)}
         .code = (((uint64){hex(i.match)}) << 32) | ({hex(i.mask)}),
     }},

core/ir/riscv64/isl/README.md

@@ -123,6 +123,7 @@ start with i, which means implicit.
 - iimm_0
 - icrs1
 - icrs1__
+- i_s_rs1_disp
 
 This maps into `riscv64_fld_t` enum in `codec.h` and `Field` enum in `codec.py`
 generator.