[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] target/riscv: write back unmodified value for csrrc/csrrs

From: Weiwei Li
Subject: Re: [PATCH v2] target/riscv: write back unmodified value for csrrc/csrrs with rs1 is not x0 but holding zero
Date: Thu, 17 Mar 2022 10:10:50 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0

在 2022/3/17 上午6:35, Alistair Francis 写道:
On Thu, Mar 17, 2022 at 1:13 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:

           riscv_raise_exception(env, ret, GETPC());
@@ -90,7 +90,7 @@ void helper_csrw_i128(CPURISCVState *env, int csr,
       RISCVException ret = riscv_csrrw_i128(env, csr, NULL,
                                             int128_make128(srcl, srch),
-                                          UINT128_MAX);
+                                          UINT128_MAX, true);

       if (ret != RISCV_EXCP_NONE) {
           riscv_raise_exception(env, ret, GETPC());
@@ -104,7 +104,7 @@ target_ulong helper_csrrw_i128(CPURISCVState *env, int csr,
       Int128 rv = int128_zero();
       RISCVException ret = riscv_csrrw_i128(env, csr, &rv,
                                             int128_make128(srcl, srch),
-                                          int128_make128(maskl, maskh));
+                                          int128_make128(maskl, maskh), true);

       if (ret != RISCV_EXCP_NONE) {
           riscv_raise_exception(env, ret, GETPC());
I am afraid the adding of "bool write_op" argument was done on many
functions, some of which do not look good to me, e.g.: predicate
funcs. v1 is much better.*>
Yeah, I agree adding  argument to predicate is not a good idea. However
it seems that the csr(like seed)

itself cannot  distinguish whether it's to be written or not except
these two ways(in v1 and v2).

Or we can take seed CSR as a special case  in riscv_csrrw_check since no
other csr will limit itself

to be accessed only with read-write instruction currently.

Or maybe, is that possible we do something in trans_csrrs() instead?
That might be a better option. Unless there are other CSRs we expect
to do this I think trans_csrr*() is probably the place to have this,
similar to the `rd == 0` case.

It could also go in helper_csrr() which would just be a simple if check.


Sorry. I don't understand what can be done in trans_csrr*(). As I said in last email: trans_csr*

have made all the read operation will go to the helper_csrr. However helper_csrr share the progress

of riscv_csrrw*  with helper_csrrw/helper_csrw to implement its function.

The truely question is that helper_csrr will call riscv_csrrw*with write_mask = zero, new_value=zero,

which cannot distinguished from the call from helper_csrrw of which its write_mask all can be zero

(origin from trans_csrrs/trans_csrrc when rs1 is not x0 and the value of rs1 reg is zero).


Weiwei Li

The read and write operation in trans_csr*  have been truely
distinguished in original code:

all the read operation will go to  the helper_csrr,  write to
helper_csrrw, read/write to helper_csrrw.

However, they all go to the same progress  riscv_csrrw* in the helper
with different arguments.


Weiwei Li


reply via email to

[Prev in Thread] Current Thread [Next in Thread]