public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andres Freund <[email protected]>
To: Niyas Sait <[email protected]>
Cc: Ian Lawrence Barwick <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Julien Rouhaud <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PATCH] Add native windows on arm64 support
Date: Sat, 5 Nov 2022 11:31:36 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAFPTBD8CCT9BNdQQ0QELoo7HPqx26_Zkn0jxg9F9u+bPz-jnEg@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<Yw/9pgS35urJZu/[email protected]>
	<[email protected]>
	<[email protected]>
	<CAB8KJ=jEZZa=2sgtpjq4raTaqh3vxx5GFFKWF=OZFHo6vp0cGg@mail.gmail.com>
	<CAFPTBD8CCT9BNdQQ0QELoo7HPqx26_Zkn0jxg9F9u+bPz-jnEg@mail.gmail.com>

Hi,

On 2022-11-03 11:06:46 +0000, Niyas Sait wrote:
> I've attached a new version of the patch which excludes the already merged
> ASLR changes and add
> small changes to handle latest changes in the build scripts.

Note that we're planning to remove the custom windows build scripts before the
next release, relying on the meson build instead.


> diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
> index 8b19ab160f..bf6a6dba35 100644
> --- a/src/include/storage/s_lock.h
> +++ b/src/include/storage/s_lock.h
> @@ -708,13 +708,21 @@ typedef LONG slock_t;
>  #define SPIN_DELAY() spin_delay()
>  
>  /* If using Visual C++ on Win64, inline assembly is unavailable.
> - * Use a _mm_pause intrinsic instead of rep nop.
> + * Use _mm_pause (x64) or __isb(arm64) intrinsic instead of rep nop.
>   */
>  #if defined(_WIN64)
>  static __forceinline void
>  spin_delay(void)
>  {
> +#ifdef _M_ARM64
> +	/*
> +	 * arm64 way of hinting processor for spin loops optimisations
> +	 * ref: https://community.arm.com/support-forums/f/infrastructure-solutions-forum/48654/ssetoneon-faq
> +	*/
> +	__isb(_ARM64_BARRIER_SY);
> +#else
>  	_mm_pause();
> +#endif
>  }
>  #else
>  static __forceinline void

I think we should just apply this, there seems very little risk of making
anything worse, given the gating to _WIN64 && _M_ARM64.


> diff --git a/src/port/pg_crc32c_armv8.c b/src/port/pg_crc32c_armv8.c
> index 9e301f96f6..981718752f 100644
> --- a/src/port/pg_crc32c_armv8.c
> +++ b/src/port/pg_crc32c_armv8.c
> @@ -14,7 +14,9 @@
>   */
>  #include "c.h"
>  
> +#ifndef _MSC_VER
>  #include <arm_acle.h>
> +#endif
>  
>  #include "port/pg_crc32c.h"

This won't suffice with the meson build, since the relevant configure test
also uses arm_acle.h:
elif host_cpu == 'arm' or host_cpu == 'aarch64'

  prog = '''
#include <arm_acle.h>

int main(void)
{
    unsigned int crc = 0;
    crc = __crc32cb(crc, 0);
    crc = __crc32ch(crc, 0);
    crc = __crc32cw(crc, 0);
    crc = __crc32cd(crc, 0);

    /* return computed value, to prevent the above being optimized away */
    return crc == 0;
}
'''

  if cc.links(prog, name: '__crc32cb, __crc32ch, __crc32cw, and __crc32cd without -march=armv8-a+crc',
      args: test_c_args)
    # Use ARM CRC Extension unconditionally
    cdata.set('USE_ARMV8_CRC32C', 1)
    have_optimized_crc = true
  elif cc.links(prog, name: '__crc32cb, __crc32ch, __crc32cw, and __crc32cd with -march=armv8-a+crc',
      args: test_c_args + ['-march=armv8-a+crc'])
    # Use ARM CRC Extension, with runtime check
    cflags_crc += '-march=armv8-a+crc'
    cdata.set('USE_ARMV8_CRC32C', false)
    cdata.set('USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK', 1)
    have_optimized_crc = true
  endif
endif

The meson checking logic is used both for msvc and other compilers, so this
will need to work with both.


> diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
> index d6bed1ce15..f1e0ff446b 100644
> --- a/src/tools/msvc/gendef.pl
> +++ b/src/tools/msvc/gendef.pl
> @@ -120,9 +120,9 @@ sub writedef
>  	{
>  		my $isdata = $def->{$f} eq 'data';
>  
> -		# Strip the leading underscore for win32, but not x64
> +		# Strip the leading underscore for win32, but not x64 and arm64
>  		$f =~ s/^_//
> -		  unless ($arch eq "x86_64");
> +		  unless ($arch ne "x86");
>  
>  		# Emit just the name if it's a function symbol, or emit the name
>  		# decorated with the DATA option for variables.
> @@ -143,7 +143,7 @@ sub writedef
>  sub usage
>  {
>  	die("Usage: gendef.pl --arch <arch> --deffile <deffile> --tempdir <tempdir> files-or-directories\n"
> -		  . "    arch: x86 | x86_64\n"
> +		  . "    arch: x86 | x86_64 | arm64 \n"
>  		  . "    deffile: path of the generated file\n"
>  		  . "    tempdir: directory for temporary files\n"
>  		  . "    files or directories: object files or directory containing object files\n"
> @@ -160,7 +160,7 @@ GetOptions(
>  	'tempdir:s' => \$tempdir,) or usage();
>  
>  usage("arch: $arch")
> -  unless ($arch eq 'x86' || $arch eq 'x86_64');
> +  unless ($arch eq 'x86' || $arch eq 'x86_64' || $arch eq 'arm64');
>  
>  my @files;
>  

Seems reasonable.

Greetings,

Andres Freund





view thread (90+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Add native windows on arm64 support
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox