[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 11/21] target/riscv: support for 128-bit bitwise instructi
From: |
Frédéric Pétrot |
Subject: |
Re: [PATCH v3 11/21] target/riscv: support for 128-bit bitwise instructions |
Date: |
Wed, 20 Oct 2021 21:18:01 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 |
Le 20/10/2021 à 19:47, Richard Henderson a écrit :
> On 10/19/21 2:48 AM, Frédéric Pétrot wrote:
>> The 128-bit bitwise instructions do not need any function prototype change
>> as the functions can be applied independently on the lower and upper part of
>> the registers.
>>
>> Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
>> Co-authored-by: Fabien Portas <fabien.portas@grenoble-inp.org>
>> ---
>> target/riscv/translate.c | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>> index e8f08f921e..71982f6284 100644
>> --- a/target/riscv/translate.c
>> +++ b/target/riscv/translate.c
>> @@ -429,6 +429,17 @@ static bool gen_logic_imm_fn(DisasContext *ctx, arg_i
>> *a,
>> DisasExtend ext,
>> gen_set_gpr(ctx, a->rd, dest);
>> + if (get_xl_max(ctx) == MXL_RV128) {
>> + if (get_ol(ctx) == MXL_RV128) {
>> + uint64_t immh = -(a->imm < 0);
>> + src1 = get_gprh(ctx, a->rs1);
>> + dest = dest_gprh(ctx, a->rd);
>> +
>> + func(dest, src1, immh);
>> + }
>> + gen_set_gprh(ctx, a->rd, dest);
>> + }
>
> If ol < RV128, you're storing the low dest into the gprh, which is wrong. It
> should be the sign-extension of the low part. But that should happen for all
> writes.
Thanks for your feedback (on the other parts too) that I'll apply.
On this specific case, in gen_set_gprh I check that the operation is not on
128 bit in which case I propagate the sign of the low part instead of using
dest (the spec says that the sign should propagate to misa_xl_max, irrelevant
of xl).
This implicitly forces the order in which the functions must be called as you
noticed, and introducing a higher level function as you suggest would indeed
make things more readable, and this can probably be applied in most places.
Frédéric
--
+---------------------------------------------------------------------------+
| Frédéric Pétrot, Pr. Grenoble INP-Ensimag/TIMA, Ensimag deputy director |
| Mob/Pho: +33 6 74 57 99 65/+33 4 76 57 48 70 Ad augusta per angusta |
| http://tima.univ-grenoble-alpes.fr frederic.petrot@univ-grenoble-alpes.fr |
+---------------------------------------------------------------------------+
- [PATCH v3 08/21] target/riscv: adding accessors to the registers upper part, (continued)
- [PATCH v3 08/21] target/riscv: adding accessors to the registers upper part, Frédéric Pétrot, 2021/10/19
- [PATCH v3 07/21] target/riscv: setup everything so that riscv128-softmmu compiles, Frédéric Pétrot, 2021/10/19
- [PATCH v3 10/21] target/riscv: support for 128-bit loads and store, Frédéric Pétrot, 2021/10/19
- [PATCH v3 12/21] target/riscv: support for 128-bit U-type instructions, Frédéric Pétrot, 2021/10/19
- [PATCH v3 11/21] target/riscv: support for 128-bit bitwise instructions, Frédéric Pétrot, 2021/10/19
- [PATCH v3 14/21] target/riscv: support for 128-bit arithmetic instructions, Frédéric Pétrot, 2021/10/19
- [PATCH v3 13/21] target/riscv: support for 128-bit shift instructions, Frédéric Pétrot, 2021/10/19
- [PATCH v3 17/21] target/riscv: helper functions to wrap calls to 128-bit csr insns, Frédéric Pétrot, 2021/10/19
- [PATCH v3 21/21] target/riscv: support for 128-bit satp, Frédéric Pétrot, 2021/10/19