Re: Use __attribute__((target(sse4.2))) for SSE42 CRC32C
Nathan Bossart <nathandbossart@gmail.com>
From: Nathan Bossart <nathandbossart@gmail.com>
To: "Devulapalli, Raghuveer" <raghuveer.devulapalli@intel.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "Shankaran, Akash" <akash.shankaran@intel.com>
Date: 2024-11-08T18:07:45Z
Lists: pgsql-hackers
On Fri, Nov 08, 2024 at 05:52:23PM +0000, Devulapalli, Raghuveer wrote:
>> So, for meson builds, we do test with and without __attribute___((target(..."))),
>> but for autoconf builds, we check for __SSE4_2__ to determine whether we need
>> a runtime check. This difference isn't the fault of your patch, but it's a little odd.
>> That being said, I'm not sure there's a problem with either approach.
>
> I just realized, isn't this a problem on MSVC? When building with MSVC,
> USE_SSE42_CRC32C is always set to true (because MSVC doesn't require a
> specific SSE42 flag to build with): see
> https://gcc.godbolt.org/z/eoc1Ec33j. This means it is always running the
> SSE42 without a runtime check which can technically SEGILL on a really
> old CPU (SSE42 is nearly 18 years old though). This problem exists in the
> master branch too.
I believe we expect MSVC builds to use meson at this point, which is
probably why there's this extra check:
if cc.get_id() == 'msvc'
cdata.set('USE_SSE42_CRC32C', false)
cdata.set('USE_SSE42_CRC32C_WITH_RUNTIME_CHECK', 1)
have_optimized_crc = true
There used to be special MSVC scripts (removed in commit 1301c80), and it
looks like those just set USE_SSE42_CRC32C_WITH_RUNTIME_CHECK
unconditionally, too.
--
nathan
Commits
-
Use __attribute__((target(...))) for SSE4.2 CRC-32C support.
- 4b03a27fafc9 18.0 landed
-
Move check for USE_AVX512_POPCNT_WITH_RUNTIME_CHECK.
- 4225276e2540 18.0 landed
-
Remove MSVC scripts
- 1301c80b2167 17.0 cited