Re: define pg_structiszero(addr, s, r)
Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Ranier Vilela <ranier.vf@gmail.com>, Peter Smith <smithpb2250@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers@lists.postgresql.org
Date: 2024-11-11T06:19:50Z
Lists: pgsql-hackers
Hi,
On Sat, Nov 09, 2024 at 04:15:04AM +0000, Bertrand Drouvot wrote:
> Hi,
>
> On Fri, Nov 08, 2024 at 11:18:09PM +1300, David Rowley wrote:
> > I tried with [1] and the
> > latest gcc does not seem to be smart enough to figure this out. I
> > tried adding some additional len checks that the compiler can use as a
> > cue and won't need to emit code for the checks providing the function
> > does get inlined. That was enough to get the compiler to not emit the
> > loops when they'll not be used. See the -DCHECK_LEN flag I'm passing
> > in the 2nd compiler window. I just don't know if putting something
> > like that into the code is a good idea as if the function wasn't
> > inlined for some reason, the extra len checks would have to be
> > compiled into the function.
> >
> > David
> >
> > [1] https://godbolt.org/z/xa81ro8GK
>
> Looking at it, that looks like an issue.
>
> I mean, without the -DCHECK_LEN flag then the SIMD code will read up to 48 bytes
> beyond the struct's memory (which is 16 bytes):
>
> This is fine:
> "
> movdqu xmm0, XMMWORD PTR [rdi]
> "
>
> But I don't think it is:
>
> "
> movdqu xmm2, XMMWORD PTR [rdi+16]
> movdqu xmm1, XMMWORD PTR [rdi+32]
> movdqu xmm3, XMMWORD PTR [rdi+48]
> "
>
> given that the struct size is only 16 bytes.
>
> Thoughts?
What about "simply" starting pg_memory_is_all_zeros() with?
"
if (len < sizeof(size_t)*8) {
while (p < end) {
if (*p++ != 0)
return false;
}
return true;
}
"
That way:
- we prevents reading beyond the memory area in the SIMD section (if < 64 bytes)
- we make sure that aligned_end can not be after the real end (could be if the
len is < 8 bytes)
- there is no need for additional size checks later in the code
- len < 64 bytes will be read byte per byte but that's likely "enough" (if not
faster) for those "small" sizes
Thoughts?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Commits
-
Use pg_memory_is_all_zeros() in PageIsVerifiedExtended()
- 03a42c9652f8 18.0 landed
-
Optimize pg_memory_is_all_zeros() in memutils.h
- 5be1dabd2ae0 18.0 landed
-
Remove use of pg_memory_is_all_zeros() in bufpage.c
- e819bbb7c82a 18.0 landed
-
Add pg_memory_is_all_zeros() in memutils.h
- 07e9e28b56db 18.0 landed