Re: [PATCH v10 03/16] target/riscv: clwz must ignore high bits (use shift-left & changed logic)

[Richard Henderson]

On 9/4/21 10:35 PM, Philipp Tomsich wrote:
> Assume clzw being executed on a register that is not sign-extended, such
> as for the following sequence that uses (1ULL << 63) | 392 as the operand
> to clzw:
>         bseti   a2, zero, 63
>         addi    a2, a2, 392
>         clzw    a3, a2
> The correct result of clzw would be 23, but the current implementation
> returns -32 (as it performs a 64bit clz, which results in 0 leading zero
> bits, and then subtracts 32).
>
> Fix this by changing the implementation to:
>   1. shift the original register up by 32
>   2. performs a target-length (64bit) clz
>   3. return 32 if no bits are set
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
> ---
>
> Changes in v10:
> - New patch, fixing correctnes for clzw called on a register with undefined
>    (as in: not properly sign-extended) upper bits.

But we have

    return gen_unary(ctx, a, EXT_ZERO, gen_clzw);

should *not* be undefined.  Ah, what's missing is

    ctx->w = true;

within trans_clzw to cause the extend to take effect.

There are a few other "w" functions that are missing that set, though they use EXT_NONE so 
there is no visible bug, it would probably be best to set w anyway.


r~