Re: define pg_structiszero(addr, s, r) - Mailing list pgsql-hackers

Hi,

On Fri, Nov 15, 2024 at 09:54:33AM -0300, Ranier Vilela wrote:
> There is a tiny typo with V13.
> + /* "len" in the [sizeof(size_t) * 8, inf] range */

I think "[sizeof(size_t) * 8, inf[ range" is correct. Infinity can not be included
into a interval.

Thinking about it, actually, "[sizeof(size_t) * 8, inf)" (note the ')' at the end)
might be the proper notation from a mathematical point of view.

> But, I'm not sure if I'm still doing something wrong.
> If so, forgive me for the noise.
>
> Of course I expected "not is_allzeros".

That's the test case which is "wrong" (not the function):

"
size_t pagebytes[BLCKSZ] = {0};
volatile bool result;

pagebytes[BLCKSZ-2] = 1;

result = pg_memory_is_all_zeros_v12(pagebytes, BLCKSZ);
"

The pagebytes is an array of size_t (8 bytes each), so the actual array size
is 8192 * 8 = 65536 bytes.

So, pagebytes[BLCKSZ-2] = 1, sets byte 65528 ((8192-2)*8) to 1.

But the function is checking up to BLCKSZ bytes (8192), so the results you
observed (which are correct).

> Anyway, I made another attempt to optimize a bit more, I don't know if it's
> safe though.

There is an issue in your v14, it calls:

"
return pg_memory_is_all_zeros_simd(ptr, ptr + len);
"

but you defined it that way:

"
static inline bool
pg_memory_is_all_zeros_simd(const size_t *p, const size_t * end)

"

while that should be:

"
static inline bool
pg_memory_is_all_zeros_simd(const void *p, const void *end)

"

Doing so, I do not observe any improvments with v14.