From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Noah Misch <noah(at)leadboat(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Remove last traces of HPPA support |
Date: | 2024-07-29 23:16:18 |
Message-ID: | 7d7294a6-f07b-46ad-bb63-a68cd8cd899c@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 30/07/2024 00:50, Thomas Munro wrote:
> On Wed, Jul 3, 2024 at 8:09 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Thomas Munro <thomas(dot)munro(at)gmail(dot)com> writes:
>>> Here are some experimental patches to try out some ideas mentioned
>>> upthread, that are approximately unlocked by that cleanup.
>>
>> FWIW, I'm good with getting rid of --disable-spinlocks and
>> --disable-atomics. That's a fair amount of code and needing to
>> support it causes problems, as you say. I am very much less
>> excited about ripping out our spinlock and/or atomics code in favor
>> of <stdatomic.h>; I just don't see the gain there, and I do see risk
>> in ceding control of the semantics and performance of those
>> primitives.
>
> OK, <stdatomic.h> part on ice for now. Here's an update of the rest,
> this time also removing the barrier fallbacks as discussed in the LTO
> thread[1].
Looks good to me.
> I guess we should also consider reimplementing the spinlock on the
> atomic API, but I can see that Andres is poking at spinlock code right
> now so I'll keep out of his way...
>
> Side issue: I noticed via CI failure when I tried to require
> read/write barriers to be provided (a choice I backed out of), that on
> MSVC we seem to be using the full memory barrier fallback for those.
> Huh? For x86, I think they should be using pg_compiler_barrier() (no
> code gen, just prevent reordering), not pg_pg_memory_barrier(), no?
Agreed, arch-x86.h is quite clear on that.
> Perhaps I'm missing something but I suspect we might be failing to
> include arch-x86.h on that compiler when we should... maybe it needs
> to detect _M_AMD64 too?
Aha, yes I think that's it. Apparently, __x86_64__ is not defined on
MSVC. To prove that, I added garbage to the "#ifdef __x86_64__" guarded
block in atomics.h. The compilation passes on MSVC, but not on other
platforms: https://cirrus-ci.com/build/6310061188841472.
That means that we're not getting the x86-64 instructions in
src/port/pg_crc32c_sse42.c on MSVC either.
I think we should do:
#ifdef _M_AMD64
#define __x86_64__
#endif
somewhere, perhaps in src/include/port/win32.h.
--
Heikki Linnakangas
Neon (https://neon.tech)
From | Date | Subject | |
---|---|---|---|
Next Message | Erik Wienhold | 2024-07-29 23:36:55 | Re: psql: Add leakproof field to \dAo+ meta-command results |
Previous Message | Sami Imseih | 2024-07-29 23:15:49 | Re: Restart pg_usleep when interrupted |