Re: [PATCH v0 1/3] bpf: Introduce tnum_scast as a tnum native sign extension helper

[Eduard Zingerman <eddyz87@gmail.com>]

On Thu, 2025-01-30 at 13:23 +0200, Dimitar Kanaliev wrote:

Hi Dimitar,

[...]

> +struct tnum tnum_scast(struct tnum a, u8 size)
> +{
> +	u64 s = size * 8 - 1;
> +	u64 sign_mask;
> +	u64 value_mask;
> +	u64 new_value, new_mask;
> +	u64 sign_bit_unknown, sign_bit_value;
> +	u64 mask;
> +
> +	if (size >= 8) {
> +		return a;
> +	}
> +
> +	sign_mask = 1ULL << s;
> +	value_mask = (1ULL << (s + 1)) - 1;
> +
> +	new_value = a.value & value_mask;
> +	new_mask = a.mask & value_mask;
> +
> +	sign_bit_unknown = (a.mask >> s) & 1;
> +	sign_bit_value = (a.value >> s) & 1;
> +
> +	mask = ~value_mask;
> +	new_mask |= mask & (0 - sign_bit_unknown);
> +	new_value |= mask & (0 - ((sign_bit_unknown ^ 1) & sign_bit_value));
> +
> +	return TNUM(new_value, new_mask);
> +}

So, effectively what you want to achieve here:
- pick a sign bit SM from mask and set signed extended bits of mask to SM
- pick a sign bit SV from value and set signed extended bits of value to SV
right?

I think this could be done a bit simpler, e.g.:

struct tnum tnum_scast(struct tnum a, u8 size)
{
	u8 s = 64 - size * 8;
	u64 value, mask;

	if (size >= 8)
		return a;

	value = ((s64)a.value << s) >> s;
	mask = ((s64)a.mask << s) >> s;
	return TNUM(value, mask);
}

wdyt?