Re: [PULL 13/17] include/exec: fix assert in size_memop

[Akihiko Odaki <akihiko.odaki@gmail.com>]

On 2025/06/06 1:26, Alex Bennée wrote:
> We can handle larger sized memops now, expand the range of the assert.
> 
> Fixes: 4b473e0c60 (tcg: Expand MO_SIZE to 3 bits)
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-ID: <20250603110204.838117-14-alex.bennee@linaro.org>
> 
> diff --git a/include/exec/memop.h b/include/exec/memop.h
> index 407a47d82c..cf7da3362e 100644
> --- a/include/exec/memop.h
> +++ b/include/exec/memop.h
> @@ -162,8 +162,8 @@ static inline unsigned memop_size(MemOp op)
>   static inline MemOp size_memop(unsigned size)
>   {
>   #ifdef CONFIG_DEBUG_TCG
> -    /* Power of 2 up to 8.  */
> -    assert((size & (size - 1)) == 0 && size >= 1 && size <= 8);
> +    /* Power of 2 up to 1024 */

I suggested using is_power_of_2() to remove the comment entirely to 
avoid hardcoding a number. Please refer to:
https://lore.kernel.org/qemu-devel/eec76ce0-c3ca-48ed-befe-e0930d4a39d9@linaro.org/

> +    assert(is_power_of_2(size) && size >= 1 && size <= (1 << MO_SIZE));

size >= 1 is unnecessary because being power of 2 implies that. The 
comment doesn't mention this condition either.

Regards,
Akihiko Odaki

>   #endif
>       return (MemOp)ctz32(size);
>   }