| Message ID | 20240216135719.1034289-2-dbarboza@ventanamicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: set vstart_eq_zero on mark_vs_dirty | expand |
On 2/16/24 03:57, Daniel Henrique Barboza wrote: > In the Vector spec section 3.2 [1]: > > "When mstatus.VS is set to Initial or Clean, executing any instruction > that changes vector state, including the vector CSRs, will change > mstatus.VS to Dirty." > > ldst_us_trans(), ldst_stride_trans(), ldst_index_trans() and > ldst_whole_trans() will change vector state regardless of being a store > op or not. Stores will set env->vstart to zero after execution (see > vext_ldst_us() in vector_helper.c), and this is vector CSR state change. In Initial or Clean state, it could be argued that vstart is already zero, so is there really a change to state? OTOH, on the exception path out of a vector store, where we *do* set vstart != 0, we do not also set vs dirty. Therefore I think that loads and stores need to manage dirty within the helper, alongside the management of vstart, or perhaps move the mark_vs_dirty call to *before* the call to the helper. r~
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc index 9e101ab434..044c9c903e 100644 --- a/target/riscv/insn_trans/trans_rvv.c.inc +++ b/target/riscv/insn_trans/trans_rvv.c.inc @@ -638,10 +638,7 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, uint32_t data, fn(dest, mask, base, tcg_env, desc); - if (!is_store) { - mark_vs_dirty(s); - } - + mark_vs_dirty(s); gen_set_label(over); return true; } @@ -799,10 +796,7 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, uint32_t rs2, fn(dest, mask, base, stride, tcg_env, desc); - if (!is_store) { - mark_vs_dirty(s); - } - + mark_vs_dirty(s); gen_set_label(over); return true; } @@ -906,10 +900,7 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1, uint32_t vs2, fn(dest, mask, base, index, tcg_env, desc); - if (!is_store) { - mark_vs_dirty(s); - } - + mark_vs_dirty(s); gen_set_label(over); return true; } @@ -1104,9 +1095,7 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, uint32_t nf, fn(dest, base, tcg_env, desc); - if (!is_store) { - mark_vs_dirty(s); - } + mark_vs_dirty(s); gen_set_label(over); return true;
In the Vector spec section 3.2 [1]: "When mstatus.VS is set to Initial or Clean, executing any instruction that changes vector state, including the vector CSRs, will change mstatus.VS to Dirty." ldst_us_trans(), ldst_stride_trans(), ldst_index_trans() and ldst_whole_trans() will change vector state regardless of being a store op or not. Stores will set env->vstart to zero after execution (see vext_ldst_us() in vector_helper.c), and this is vector CSR state change. [1] https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#vector-start-index-csr-vstart Fixes: 8e1ee1fb57 ("target/riscv: rvv-1.0: add translation-time vector context status") Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/insn_trans/trans_rvv.c.inc | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-)