pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: John Naylor <johncnaylorls@gmail.com>
Cc: Ants Aasma <ants@cybertec.at>
Cc: pgsql-hackers@postgresql.org
Subject: Re: add AVX2 support to simd.h
Date: Wed, 27 Mar 2024 13:57:16 -0500
Message-ID: <20240327185716.GA4188857@nathanxps13> (raw)
In-Reply-To: <3206056.1711504137@sss.pgh.pa.us>
References: <CANWCAZYfUv3iN2Vx--tmQP9WU6xwfU8d=gA7LoFXOYP9-wo8Hg@mail.gmail.com>
	<20240321170944.GA1767527@nathanxps13>
	<CANWCAZb7TGEk6ib12Rr6pbGMy9pqofT0dS=8Dkm+=MwJQZAEww@mail.gmail.com>
	<20240325152121.GB2662364@nathanxps13>
	<20240325213754.GA3094030@nathanxps13>
	<20240326190904.GA3700674@nathanxps13>
	<3189995.1711495704@sss.pgh.pa.us>
	<20240326235554.GA3766883@nathanxps13>
	<20240327013616.GA3940109@nathanxps13>
	<3206056.1711504137@sss.pgh.pa.us>

On Tue, Mar 26, 2024 at 09:48:57PM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> I just did the minimal fix for now, i.e., I moved the new label into the
>> SIMD section of the function.  I think it would be better stylistically to
>> move the one-by-one logic to an inline helper function, but I didn't do
>> that just in case it might negatively impact performance.  I'll look into
>> this and will follow up with another patch if it looks good.
> 
> Sounds like a plan.

Here's what I had in mind.  My usual benchmark seems to indicate that this
shouldn't impact performance.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachments:

v1-0001-improve-style-of-pg_lfind32.patch (text/x-diff, 2.8K) download From c3f163753246c5ec82dd8c5dba70232cbeebbf2a Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nathan@postgresql.org> Date: Wed, 27 Mar 2024 13:50:17 -0500 Subject: [PATCH v1 1/1] improve style of pg_lfind32() --- src/include/port/pg_lfind.h | 58 +++++++++++++++---------------------- 1 file changed, 24 insertions(+), 34 deletions(-) diff --git a/src/include/port/pg_lfind.h b/src/include/port/pg_lfind.h index 33e8471b03..5b76cc8937 100644 --- a/src/include/port/pg_lfind.h +++ b/src/include/port/pg_lfind.h @@ -80,6 +80,24 @@ pg_lfind8_le(uint8 key, uint8 *base, uint32 nelem) return false; } +/* + * pg_lfind32_one_by_one_helper + * + * Searches the array of integers one-by-one. The caller is responsible for + * ensuring that there are at least "nelem" integers in the array. + */ +static inline bool +pg_lfind32_one_by_one_helper(uint32 key, uint32 *base, uint32 nelem) +{ + for (int i = 0; i < nelem; i++) + { + if (key == base[i]) + return true; + } + + return false; +} + #ifndef USE_NO_SIMD /* * pg_lfind32_simd_helper @@ -134,9 +152,8 @@ pg_lfind32_simd_helper(const Vector32 keys, uint32 *base) static inline bool pg_lfind32(uint32 key, uint32 *base, uint32 nelem) { - uint32 i = 0; - #ifndef USE_NO_SIMD + uint32 i = 0; /* * For better instruction-level parallelism, each loop iteration operates @@ -150,25 +167,15 @@ pg_lfind32(uint32 key, uint32 *base, uint32 nelem) const uint32 tail_idx = nelem & ~(nelem_per_iteration - 1); #if defined(USE_ASSERT_CHECKING) - bool assert_result = false; - - /* pre-compute the result for assert checking */ - for (int j = 0; j < nelem; j++) - { - if (key == base[j]) - { - assert_result = true; - break; - } - } + bool assert_result = pg_lfind32_one_by_one_helper(key, base, nelem); #endif /* - * If there aren't enough elements for the SIMD code, jump to the standard + * If there aren't enough elements for the SIMD code, use the standard * one-by-one linear search code. */ if (nelem < nelem_per_iteration) - goto one_by_one; + return pg_lfind32_one_by_one_helper(key, base, nelem); /* * Process as many elements as possible with a block of 4 registers. @@ -193,27 +200,10 @@ pg_lfind32(uint32 key, uint32 *base, uint32 nelem) */ Assert(assert_result == pg_lfind32_simd_helper(keys, &base[nelem - nelem_per_iteration])); return pg_lfind32_simd_helper(keys, &base[nelem - nelem_per_iteration]); - -one_by_one: - -#endif /* ! USE_NO_SIMD */ - +#else /* Process the elements one at a time. */ - for (; i < nelem; i++) - { - if (key == base[i]) - { -#ifndef USE_NO_SIMD - Assert(assert_result == true); + return pg_lfind32_one_by_one_helper(key, base, nelem); #endif - return true; - } - } - -#ifndef USE_NO_SIMD - Assert(assert_result == false); -#endif - return false; } #endif /* PG_LFIND_H */ -- 2.25.1

view thread (48+ messages)  latest in thread

Message-ID: <20240327185716.GA4188857@nathanxps13>
Permalink:  ../20240327185716.GA4188857@nathanxps13/
Also on:    postgresql.org/message-id/20240327185716.GA4188857@nathanxps13

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: nathandbossart@gmail.com, tgl@sss.pgh.pa.us, johncnaylorls@gmail.com, ants@cybertec.at
  Subject: Re: add AVX2 support to simd.h
  In-Reply-To: <20240327185716.GA4188857@nathanxps13>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox