[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] RISC-V: Fix riscv_isa_string, use popcount t
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH v2] RISC-V: Fix riscv_isa_string, use popcount to count bits |
Date: |
Sat, 10 Mar 2018 22:25:35 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
On 03/09/2018 10:01 PM, Michael Clark wrote:
> Logic bug caused the string size calculation for the RISC-V
> format ISA string to be small. This fix allows slack for rv128.
>
> Cc: Palmer Dabbelt <address@hidden>
> Cc: Peter Maydell <address@hidden>
> Cc: Eric Blake <address@hidden>
> Signed-off-by: Michael Clark <address@hidden>
> ---
> target/riscv/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 4851890..1456535 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -391,7 +391,7 @@ static const TypeInfo riscv_cpu_type_info = {
> char *riscv_isa_string(RISCVCPU *cpu)
> {
> int i;
> - size_t maxlen = 5 + ctz32(cpu->env.misa);
> + size_t maxlen = 8 + ctpop64(cpu->env.misa);
Can you document the magic 5/8?
This looks nice, but this seems to me too much optimization to save few
bytes, using sizeof(riscv_exts) is overflow-free.
Maybe this is enough and self-explanatory:
const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts);
> char *isa_string = g_new0(char, maxlen);
> snprintf(isa_string, maxlen, "rv%d", TARGET_LONG_BITS);
Also, if you keep the snprintf() return value, you can (naming it 'n')
simplify (also easier to review):
> for (i = 0; i < sizeof(riscv_exts); i++) {
>
if (cpu->env.misa & RV(riscv_exts[i])) {
- isa_string[strlen(isa_string)] = riscv_exts[i] - 'A' + 'a';
+ isa_string[n++] = tolower(riscv_exts[i]);
}
}
and simply use g_new() with:
+ isa_string[n] = '\0';
return isa_string;
}
Regards,
Phil.