Re: Use __attribute__((target(sse4.2))) for SSE42 CRC32C

From: Nathan Bossart <nathandbossart(at)gmail(dot)com>
To: "Devulapalli, Raghuveer" <raghuveer(dot)devulapalli(at)intel(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Shankaran, Akash" <akash(dot)shankaran(at)intel(dot)com>
Subject: Re: Use __attribute__((target(sse4.2))) for SSE42 CRC32C
Date: 2024-11-08 18:07:45
Message-ID: Zy5Tceeb4Ybfx39q@nathan
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Maxim Orlov 2024-11-08 18:10:28 Re: POC: make mxidoff 64 bits
Previous Message Devulapalli, Raghuveer 2024-11-08 17:52:23 RE: Use __attribute__((target(sse4.2))) for SSE42 CRC32C