Date: Tue, 2 Jan 2024 16:00:18 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: John Naylor <johncnaylorls@gmail.com>, Ants Aasma <ants@cybertec.at>,
	pgsql-hackers@postgresql.org
Subject: Re: add AVX2 support to simd.h
Message-ID: <20240102220018.GB1063425@nathanxps13>
References: <20231129171526.GA857928@nathanxps13>
 <CANWCAZZHEHufYD7nPNX6sPZ1mHo+30qTiV5UPXL_evh-N=1n5g@mail.gmail.com>
 <20240102161123.GA955987@nathanxps13>
 <4105101.1704217804@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4105101.1704217804@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/20240102220018.GB1063425%40nathanxps13>
Precedence: bulk

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