Add Zilsd/Zclsd Support #765
Conversation
model/riscv_insts_zilsd.sail
Outdated
| match ext_data_get_addr(rs1, offset, Read(Data), width_bytes) { | ||
| Ext_DataAddr_Error(e) => { ext_handle_data_check_error(e); RETIRE_FAIL }, | ||
| Ext_DataAddr_OK(vaddr) => | ||
| if check_misaligned(vaddr, width) | ||
| then { handle_mem_exception(vaddr, E_Load_Addr_Align()); RETIRE_FAIL } | ||
| else match translateAddr(vaddr, Read(Data)) { | ||
| TR_Failure(e, _) => { handle_mem_exception(vaddr, e); RETIRE_FAIL }, | ||
| TR_Address(paddr, _) => | ||
| match mem_read(Read(Data), paddr, 8, false, false, false) { | ||
| Ok(result) => { | ||
| X(rd) = result[31..0]; | ||
| X(rd + 1) = result[63..32]; | ||
| RETIRE_SUCCESS | ||
| }, | ||
| Err(e) => { handle_mem_exception(vaddr, e); RETIRE_FAIL }, | ||
| }, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm curious, wouldn't it make more sense to replace LOAD_RV32 (and STORE_RV32) with two sequential 32-bit load/store operations instead of one 64-bit load/store operation?
Most 32-bit systems have a 32-bit memory bus, so performing two 32-bit accesses would be more realistic than a single 64-bit access on such hardware.
This implementation would also allow us to test the more interesting case where interrupts could occur between the two operations, as explicitly permitted by the Zilsd specification.
There was a problem hiding this comment.
That's definitely a good idea, I didn't think of that
| /* SPDX-License-Identifier: BSD-2-Clause */ | ||
| /*=======================================================================================*/ | ||
|
|
||
| function clause extensionEnabled(Ext_Zclsd) = true & extensionEnabled(Ext_Zilsd) & extensionEnabled(Ext_Zca) & not(extensionEnabled(Ext_Zcf)) & xlen == 32 |
There was a problem hiding this comment.
This seems to create a mutual cycle between extensionEnabled(Ext_Zclsd) and extensionEnabled(Ext_Zcf).
| function clause extensionEnabled(Ext_Zilsd) = true & xlen == 32 | ||
|
|
||
| /* ****************************************************************** */ | ||
| union clause ast = LOAD_RV32 : (bits(12), regidx, regidx) |
There was a problem hiding this comment.
LOAD_DOUBLE might be a better name for the AST constructor than LOAD_RV32. It's unfortunate that the spec re-uses LOAD. (Could we ask the spec authors to change this before ratification?) Similarly for STORE_RV32.
|
|
||
| function clause execute LOAD_RV32(imm, rs1, rd) = { | ||
| // Use of misaligned (odd-numbered) registers is reserved. | ||
| assert(regidx_to_regno(rd) % 2 == 0); |
There was a problem hiding this comment.
Instead of asserting here, it would be better to put this constraint on rd in the encdec clause. Similarly for rs2 in the store instruction.
|
how does this relate to #490? |
spec: https://github.com/riscv/riscv-zilsd/blob/main/zilsd.adoc#insns-sd
function clause extensionEnabled(Ext_Zclsd) = true & xxxis a placeholder, waiting for config system