From: Tom Lane <tgl@sss.pgh.pa.us>
To: Nathan Bossart <nathandbossart@gmail.com>
cc: John Naylor <johncnaylorls@gmail.com>, Ants Aasma <ants@cybertec.at>,
        pgsql-hackers@postgresql.org
Subject: Re: add AVX2 support to simd.h
In-reply-to: <20240102161123.GA955987@nathanxps13>
References: <20231129171526.GA857928@nathanxps13> <CANWCAZZHEHufYD7nPNX6sPZ1mHo+30qTiV5UPXL_evh-N=1n5g@mail.gmail.com> <20240102161123.GA955987@nathanxps13>
Comments: In-reply-to Nathan Bossart <nathandbossart@gmail.com>
	message dated "Tue, 02 Jan 2024 10:11:23 -0600"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <4105100.1704217804.1@sss.pgh.pa.us>
Date: Tue, 02 Jan 2024 12:50:04 -0500
Message-ID: <4105101.1704217804@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/4105101.1704217804%40sss.pgh.pa.us>
Precedence: bulk

Nathan Bossart <nathandbossart@gmail.com> writes:
> I'm tempted to propose that we move forward with this patch as-is after
> adding a buildfarm machine that compiles with -mavx2 or -march=x86-64-v3.
> There is likely still follow-up work to make these improvements more
> accessible, but I'm not sure that is a strict prerequisite here.

The patch needs better comments (as in, more than "none whatsoever").
It doesn't need to be much though, perhaps like

+#if defined(__AVX2__)
+
+/*
+ * When compiled with -mavx2 or allied options, we prefer AVX2 instructions.
+ */
+#include <immintrin.h>
+#define USE_AVX2
+typedef __m256i Vector8;
+typedef __m256i Vector32;

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?

			regards, tom lane