From 61cfbf637387d8909664ccd3c76eb17238d62301 Mon Sep 17 00:00:00 2001 From: "Paul A. Clarke" Date: Mon, 13 Nov 2023 20:21:49 -0600 Subject: [PATCH] Fix Store-Conditional assembly operand order and add parens The operand order for Store-Conditional assembly has the second and third operands reversed. The RISC-V Instruction Set Manual states: > ``` > SC.W conditionally writes a word in rs2 to the address in rs1 [...] > If the SC.W succeeds, the instruction writes the word in rs2 to memory, > and it writes zero to rd. If the SC.W fails, the instruction does not > write to memory, and it writes a nonzero value to rd. > ``` `rd` is for the return code, `rs2` is the value, and `rs1` is the memory address. For the syntax `sc.w A,B,(C)`: - `A` is where the result is stored, per convention. So, this is `rd`. - `B` is the value to be stored. So, this is `rs2`. - `C` is the address at which to store the value. So, this is `rs1`. The resulting syntax would be `stc.w rd,rs2,(rs1)`. The current assembly representation is: ``` "sc." ^ size_mnemonic(size) [...] reg_name(rd) ^ sep() ^ reg_name(rs1) ^ sep() ^ reg_name(rs2) ``` Note that the order is wrong. In addition, parentheses are missing around `rs2`. Fix this instance, as well as two other instances where parentheses are missing. Fixes #338. Fixes #344. Suggested-by: Tim Hutt --- model/riscv_insts_aext.sail | 4 ++-- model/riscv_insts_base.sail | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/model/riscv_insts_aext.sail b/model/riscv_insts_aext.sail index ff7dc746e..6a329e110 100644 --- a/model/riscv_insts_aext.sail +++ b/model/riscv_insts_aext.sail @@ -165,7 +165,7 @@ function clause execute(LOADRES(aq, rl, rs1, width, rd)) = { } mapping clause assembly = LOADRES(aq, rl, rs1, size, rd) - <-> "lr." ^ size_mnemonic(size) ^ maybe_aq(aq) ^ maybe_rl(rl) ^ spc() ^ reg_name(rd) ^ sep() ^ reg_name(rs1) + <-> "lr." ^ size_mnemonic(size) ^ maybe_aq(aq) ^ maybe_rl(rl) ^ spc() ^ reg_name(rd) ^ sep() ^ "(" ^ reg_name(rs1) ^ ")" /* ****************************************************************** */ union clause ast = STORECON : (bool, bool, regidx, regidx, word_width, regidx) @@ -251,7 +251,7 @@ function clause execute (STORECON(aq, rl, rs2, rs1, width, rd)) = { } mapping clause assembly = STORECON(aq, rl, rs2, rs1, size, rd) - <-> "sc." ^ size_mnemonic(size) ^ maybe_aq(aq) ^ maybe_rl(rl) ^ spc() ^ reg_name(rd) ^ sep() ^ reg_name(rs1) ^ sep() ^ reg_name(rs2) + <-> "sc." ^ size_mnemonic(size) ^ maybe_aq(aq) ^ maybe_rl(rl) ^ spc() ^ reg_name(rd) ^ sep() ^ reg_name(rs2) ^ sep() ^ "(" ^ reg_name(rs1) ^ ")" /* ****************************************************************** */ union clause ast = AMO : (amoop, bool, bool, regidx, regidx, word_width, regidx) diff --git a/model/riscv_insts_base.sail b/model/riscv_insts_base.sail index 30e886a71..eec5fbdc7 100644 --- a/model/riscv_insts_base.sail +++ b/model/riscv_insts_base.sail @@ -154,7 +154,7 @@ mapping clause encdec = RISCV_JALR(imm, rs1, rd) <-> imm @ rs1 @ 0b000 @ rd @ 0b1100111 mapping clause assembly = RISCV_JALR(imm, rs1, rd) - <-> "jalr" ^ spc() ^ reg_name(rd) ^ sep() ^ reg_name(rs1) ^ sep() ^ hex_bits_12(imm) + <-> "jalr" ^ spc() ^ reg_name(rd) ^ sep() ^ hex_bits_12(imm) ^ "(" ^ reg_name(rs1) ^ ")" /* see riscv_jalr_seq.sail or riscv_jalr_rmem.sail for the execute clause. */ @@ -430,7 +430,7 @@ mapping maybe_u = { } mapping clause assembly = LOAD(imm, rs1, rd, is_unsigned, size, aq, rl) - <-> "l" ^ size_mnemonic(size) ^ maybe_u(is_unsigned) ^ maybe_aq(aq) ^ maybe_rl(rl) ^ spc() ^ reg_name(rd) ^ sep() ^ hex_bits_12(imm) ^ opt_spc() ^ "(" ^ opt_spc() ^ reg_name(rs1) ^ opt_spc() ^ ")" + <-> "l" ^ size_mnemonic(size) ^ maybe_u(is_unsigned) ^ maybe_aq(aq) ^ maybe_rl(rl) ^ spc() ^ reg_name(rd) ^ sep() ^ hex_bits_12(imm) ^ "(" ^ reg_name(rs1) ^ ")" /* ****************************************************************** */ union clause ast = STORE : (bits(12), regidx, regidx, word_width, bool, bool)