Re: a couple of small patches for simd.h

0001 essentially reverts commit c6a43c2, and instead fixes the problem
(MSVC dislikes casts to the same type) by omitting the cast in the
problematic line in pg_lfind32(). While working on optimizing hex_encode()
and hex_decode() [0], I noticed that implicit conversions sufficed.

0002 optimizes vector8_has_le() on AArch64 by using vminvq_u8(). I needed
vector8_has_ge() for the hex_encode()/hex_decode() work and noticed this
opportunity.

[0] https://commitfest.postgresql.org/patch/5538/

--
nathan

On Tue, Sep 23, 2025 at 4:44 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> 0001 essentially reverts commit c6a43c2, and instead fixes the problem
> (MSVC dislikes casts to the same type) by omitting the cast in the
> problematic line in pg_lfind32(). While working on optimizing hex_encode()
> and hex_decode() [0], I noticed that implicit conversions sufficed.

I don't remember why that cast was there, but I suggest testing on gcc
/ aarch64 if you haven't already.

As for 0002, +1 for more succinct expressions when the hardware supports it.

--
John Naylor
Amazon Web Services

On Wed, Sep 24, 2025 at 08:04:59AM +0700, John Naylor wrote:
> On Tue, Sep 23, 2025 at 4:44 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>> 0001 essentially reverts commit c6a43c2, and instead fixes the problem
>> (MSVC dislikes casts to the same type) by omitting the cast in the
>> problematic line in pg_lfind32(). While working on optimizing hex_encode()
>> and hex_decode() [0], I noticed that implicit conversions sufficed.
>
> I don't remember why that cast was there, but I suggest testing on gcc
> / aarch64 if you haven't already.

Ah, I thought CI was testing that, but apparently it is not. After some
basic testing in godbolt.org, I see that leaving out the cast makes
gcc/arm64 unhappy, and adding the cast makes msvc/x64 unhappy. gcc has a
-flax-vector-conversions option that fixes it, but the documentation for
that option [0] cautions against using it. So, 0001 is bogus, and I need
to figure out how to get the hex_{encode,decode} patches working for
gcc/arm64...

[0] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html#index-flax-vector-conversions

--
nathan

Here's a new version of 0002 with a modified SSE2 implementation, as
discussed elsewhere [0]. This allows us to remove vector8_ssub().

[0] https://postgr.es/m/aNWO7L43UevRErw_%40nathan

--
nathan

On Thu, Sep 25, 2025 at 02:10:19PM -0500, Nathan Bossart wrote:
> Here's a new version of 0002 with a modified SSE2 implementation, as
> discussed elsewhere [0]. This allows us to remove vector8_ssub().

Sorry for the noise. v3 fixes the mixed-declarations-and-code problems.

--
nathan

On Thu, Sep 25, 2025 at 04:27:35PM -0500, Nathan Bossart wrote:
> On Thu, Sep 25, 2025 at 02:10:19PM -0500, Nathan Bossart wrote:
>> Here's a new version of 0002 with a modified SSE2 implementation, as
>> discussed elsewhere [0]. This allows us to remove vector8_ssub().
>
> Sorry for the noise. v3 fixes the mixed-declarations-and-code problems.

Barring objections, I plan to commit this soon. Probably tomorrow.

--
nathan

On Fri, Oct 3, 2025 at 12:36 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
> Barring objections, I plan to commit this soon. Probably tomorrow.

LGTM.

--
John Naylor
Amazon Web Services

On Fri, Oct 03, 2025 at 02:40:22PM +0700, John Naylor wrote:
> LGTM.

Committed, thanks for reviewing.

--
nathan