public inbox for [email protected]  

help / color / mirror / Atom feed

From: Nathan Bossart <[email protected]>
To: Tom Lane <[email protected]>
Cc: John Naylor <[email protected]>
Cc: Ants Aasma <[email protected]>
Cc: [email protected]
Subject: Re: add AVX2 support to simd.h
Date: Tue, 2 Jan 2024 16:00:18 -0600
Message-ID: <20240102220018.GB1063425@nathanxps13> (raw)
In-Reply-To: <[email protected]>
References: <20231129171526.GA857928@nathanxps13>
	<CANWCAZZHEHufYD7nPNX6sPZ1mHo+30qTiV5UPXL_evh-N=1n5g@mail.gmail.com>
	<20240102161123.GA955987@nathanxps13>
	<[email protected]>

On Tue, Jan 02, 2024 at 12:50:04PM -0500, Tom Lane wrote:
> The patch needs better comments (as in, more than "none whatsoever").

Yes, will do.

> Also, do you really want to structure the header so that USE_SSE2
> doesn't get defined?  In that case you are committing to provide
> an AVX2 replacement every single place that there's USE_SSE2, which
> doesn't seem like a great thing to require.  OTOH, maybe there's
> no choice given than we need a different definition for Vector8 and
> Vector32?

Yeah, the precedent is to use these abstracted types elsewhere so that any
SIMD-related improvements aren't limited to one architecture.  There are a
couple of places that do explicitly check for USE_NO_SIMD, though.  Maybe
there's an eventual use-case for using SSE2 intrinsics even when you have
AVX2 support, but for now, ensuring we have an AVX2 replacement for
everything doesn't seem particularly burdensome.

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

view thread (10+ messages)  latest in thread

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: add AVX2 support to simd.h
  In-Reply-To: <20240102220018.GB1063425@nathanxps13>

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

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