Re: Remove last traces of HPPA support

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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-30 00:39:44
Message-ID: CA+hUKGJckzLQ=vLc7nWBkJroFc2WVOimdMnT8Cdj+_=K68skdA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 30, 2024 at 11:16 AM Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> 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:
> > 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.

Thanks. I'll wait just a bit longer to see if anyone else has comments.

> > 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.

Hmm. I had come up with the opposite solution, because we already
tested for _M_AMD64 explicitly elsewhere, and also I was thinking we
would back-patch, and I don't want to cause problems for external code
that thinks that __x86_64__ implies it can bust out some GCC inline
assembler or something. But I don't have a strong opinion, your idea
is certainly simpler to implement and I also wouldn't mind much if we
just fixed it in master only, for fear of subtle breakage...

Same problem probably exists for i386. I don't think CI, build farm
or the EDB packaging team do 32 bit Windows, so that makes it a little
hard to know if your blind code changes have broken or fixed
anything... on the other hand it's pretty simple...

I wondered if the pre-Meson system might have somehow defined
__x86_64__, but I'm not seeing it. Commit b64d92f1a56 explicitly
mentions that it was tested on MSVC, so I guess maybe it was just
always "working" but not quite taking the intended code paths? Funny
though, that code that calls _mm_pause() on AMD64 or the __asm thing
that only works on i386 doesn't look like blind code to me. Curious.

Attachment Content-Type Size
0001-Fix-x86-architecture-detection-on-MSVC.patch text/x-patch 3.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Sutou Kouhei 2024-07-30 02:58:24 Re: Make COPY format extendable: Extract COPY TO format implementations
Previous Message Euler Taveira 2024-07-30 00:05:26 Re: speed up a logical replica setup