Date: Sat, 27 Aug 2022 15:12:34 -0700
From: Nathan Bossart <nathandbossart@gmail.com>
To: John Naylor <john.naylor@enterprisedb.com>
Cc: Andres Freund <andres@anarazel.de>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: use ARM intrinsics in pg_lfind32() where available
Message-ID: <20220827221234.GA15951@nathanxps13>
References: <20220822211547.GA1126462@nathanxps13>
 <CAFBsxsEN5nW3uRh=jrs-QexDrC1btu0ZfriD3FFfb=3J6tAngg@mail.gmail.com>
 <20220824180111.GB1302810@nathanxps13>
 <CAFBsxsE-mCC4Dxb6wmgEZTynyKtER0Y0ha_Rw_rLMY_u2j2oqg@mail.gmail.com>
 <20220825045729.GA1458024@nathanxps13>
 <CAFBsxsHa9QmLk33c5C-12Uuh35Ofv+qzbT+auGrqC3fQCorqBg@mail.gmail.com>
 <20220826045115.GA1638993@nathanxps13>
 <20220826061347.GA1777731@nathanxps13>
 <20220826182403.GA1917683@nathanxps13>
 <CAFBsxsF=K1Vo4fZELJMCSF8GGbcrC_FnkMt3sQcBZt=MExmjPA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAFBsxsF=K1Vo4fZELJMCSF8GGbcrC_FnkMt3sQcBZt=MExmjPA@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/20220827221234.GA15951%40nathanxps13>
Precedence: bulk

Thanks for taking a look.

On Sat, Aug 27, 2022 at 01:59:06PM +0700, John Naylor wrote:
> I don't forsee any use of emulating vector registers with uint64 if
> they only hold two ints. I wonder if it'd be better if all vector32
> functions were guarded with #ifndef NO_USE_SIMD. (I wonder if
> declarations without definitions cause warnings...)

Yeah.  I was a bit worried about the readability of this file with so many
#ifndefs, but after trying it out, I suppose it doesn't look _too_ bad.

> + * NB: This function assumes that each lane in the given vector either has all
> + * bits set or all bits zeroed, as it is mainly intended for use with
> + * operations that produce such vectors (e.g., vector32_eq()).  If this
> + * assumption is not true, this function's behavior is undefined.
> + */
> 
> Hmm?

Yup.  The problem is that AFAICT there's no equivalent to
_mm_movemask_epi8() on aarch64, so you end up with something like

	vmaxvq_u8(vandq_u8(v, vector8_broadcast(0x80))) != 0

But for pg_lfind32(), we really just want to know if any lane is set, which
only requires a call to vmaxvq_u32().  I haven't had a chance to look too
closely, but my guess is that this ultimately results in an extra AND
operation in the aarch64 path, so maybe it doesn't impact performance too
much.  The other option would be to open-code the intrinsic function calls
into pg_lfind.h.  I'm trying to avoid the latter, but maybe it's the right
thing to do for now...  What do you think?

> -#elif defined(USE_SSE2)
> +#elif defined(USE_SSE2) || defined(USE_NEON)
> 
> I think we can just say #else.

Yes.

> -#if defined(USE_SSE2)
> - __m128i sub;
> +#ifndef USE_NO_SIMD
> + Vector8 sub;
> 
> +#elif defined(USE_NEON)
> +
> + /* use the same approach as the USE_SSE2 block above */
> + sub = vqsubq_u8(v, vector8_broadcast(c));
> + result = vector8_has_zero(sub);
> 
> I think we should invent a helper that does saturating subtraction and
> call that, inlining the sub var so we don't need to mess with it
> further.

Good idea, will do.

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