Re: [PATCH] Add native windows on arm64 support
Hello, Just wanted to give a heads up that I am moving to a new role outside Linaro and as a result wouldn't be able to continue contributing. Anthony Roberts from Linaro is going to support the enablement work. -- Niyas
Re: [PATCH] Add native windows on arm64 support
On 11/05/2023 00:34, Michael Paquier wrote: On Wed, May 10, 2023 at 09:24:39AM -0400, Andrew Dunstan wrote: We will definitely want buildfarm support. I don't have such a machine and am not likely to have one any time soon. I do run drongo and fairywren on an EC2 instance, but AWS doesn't seem to have support for Windows on ARM. Maybe it's available on Azure (maybe then some of our colleagues working at Microsoft could arrange such a beast for me to set up potential buildfarm animal, or else run one themselves.) Unfortunately AWS does not offer ARM on Windows with an EC2, that's something I have also looked at setting up :/ Anyway, Niyat has mentioned me that he was working on that, but it seems that it was off-list. Niyat, could you confirm this part? -- Michael If we have Azure VM credits, we could use that to deploy buildfarm machines. Otherwise, I can try to allocate a machine from Linaro Windows Lab for the buildbot. -- Niyas
Re: [PATCH] Add native windows on arm64 support
On 19/01/2023 10:09, Niyas Sait wrote:
On 17/01/2023 22:51, Andres Freund wrote:
int main(void)
@@ -1960,18 +1966,19 @@ int main(void)
}
'''
- 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
+ if cc.links(prog, name: '__crc32cb, __crc32ch, __crc32cw, and
__crc32cd without -march=armv8-a+crc',
+ args: test_c_args)
Seems like it'd be easier to read if you don't re-indent this, but
just have
the cc.get_id() == 'msvc' part of this if/else-if.
I've attached a new version (v8) to fix the above indentation issue.
Could someone please help with the review ?
--
NiyasFrom 108ad061caa15c7346d53e906794c4e971afbcc0 Mon Sep 17 00:00:00 2001
From: Niyas Sait
Date: Fri, 16 Dec 2022 10:45:56 +
Subject: [PATCH v8] Enable postgres native build for windows-arm64 platform
- Add support for meson build
- Add arm64 definition of spin_delay function
- Exclude arm_acle.h import for MSVC
---
doc/src/sgml/install-windows.sgml | 3 ++-
meson.build | 11 +--
src/include/storage/s_lock.h | 20 ++--
src/port/pg_crc32c_armv8.c| 2 ++
src/tools/msvc/gendef.pl | 8
5 files changed, 35 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/install-windows.sgml
b/doc/src/sgml/install-windows.sgml
index cbc70a039c..2ecd5fcf38 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -352,7 +352,8 @@ $ENV{MSBFLAGS}="/m";
Special Considerations for 64-Bit Windows
- PostgreSQL will only build for the x64 architecture on 64-bit Windows.
+ PostgreSQL will only build for the x64 and ARM64 architectures on 64-bit
+ Windows.
diff --git a/meson.build b/meson.build
index 096044628c..6cca212fae 100644
--- a/meson.build
+++ b/meson.build
@@ -2045,8 +2045,11 @@ int main(void)
elif host_cpu == 'arm' or host_cpu == 'aarch64'
prog = '''
+#ifdef _MSC_VER
+#include
+#else
#include
-
+#endif
int main(void)
{
unsigned int crc = 0;
@@ -2060,7 +2063,11 @@ int main(void)
}
'''
- if cc.links(prog, name: '__crc32cb, __crc32ch, __crc32cw, and __crc32cd
without -march=armv8-a+crc',
+ if cc.get_id() == 'msvc'
+cdata.set('USE_ARMV8_CRC32C', false)
+cdata.set('USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK', 1)
+have_optimized_crc = true
+ elif 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)
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index c9fa84cc43..a7973eae49 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -707,15 +707,31 @@ 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.
+/*
+ * If using Visual C++ on Win64, inline assembly is unavailable.
+ * Use architecture specific intrinsics.
*/
#if defined(_WIN64)
+/*
+ * For Arm64, use __isb intrinsic. See aarch64 inline assembly definition for
details.
+ */
+#ifdef _M_ARM64
+static __forceinline void
+spin_delay(void)
+{
+/* Reference:
https://learn.microsoft.com/en-us/cpp/intrinsics/arm64-intrinsics#BarrierRestrictions
*/
+ __isb(_ARM64_BARRIER_SY);
+}
+#else
+/*
+ * For x64, use _mm_pause intrinsic instead of rep nop.
+ */
static __forceinline void
spin_delay(void)
{
_mm_pause();
}
+#endif
#else
static __forceinline void
spin_delay(void)
diff --git a/src/port/pg_crc32c_armv8.c b/src/port/pg_crc32c_armv8.c
index d8fae510cf..3d7eb748ff 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
+#endif
#include "port/pg_crc32c.h"
diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
index e7cbefcbc3..934dc17b96 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';
- # S
Re: [PATCH] Add native windows on arm64 support
On 17/01/2023 22:51, Andres Freund wrote:
Hi,
On 2022-12-16 10:52:23 +, Niyas Sait wrote:
Subject: [PATCH v7] Enable postgres native build for windows-arm64 platform
elif host_cpu == 'arm' or host_cpu == 'aarch64'
- prog = '''
+ if cc.get_id() == 'msvc'
+cdata.set('USE_ARMV8_CRC32C', false)
+cdata.set('USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK', 1)
+have_optimized_crc = true
I dimly recall that windows might actually require the relevant extension on
arm?
Do you mean we don't need the runtime checks for CRC ?
CRC is an optional extension for ARMv8 base architecture. I am not sure
if windows make it mandatory to have this implementation.
+ else
+prog = '''
#include
I'd just make this include #ifdef _MSV_VER (or whatever it is).
The code snippet is not used for MSVC part. I am not sure why we need to
add the #ifdef _MSC_VER.
int main(void)
@@ -1960,18 +1966,19 @@ int main(void)
}
'''
- 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
+if cc.links(prog, name: '__crc32cb, __crc32ch, __crc32cw, and __crc32cd
without -march=armv8-a+crc',
+args: test_c_args)
Seems like it'd be easier to read if you don't re-indent this, but just have
the cc.get_id() == 'msvc' part of this if/else-if.
Yes that looks better. will do it in next patch.
Thanks Andres for the review.
--
Niyas
Re: [PATCH] Add native windows on arm64 support
On 16/12/2022 10:52, Niyas Sait wrote: On 12/12/2022 23:56, Michael Paquier wrote: On Mon, Dec 12, 2022 at 01:38:37PM +, Niyas Sait wrote: On 05/12/2022 18:14, Andres Freund wrote: I think the old build system specific part is really minimal in the patch. I can strip out those if that's preferred. Removing all the changes from src/tools/msvc/ is an approach that works for me. I've attached a new version (v7) that removes changes to support old MSVC build system. Hello, Can I get review for the latest patch please ? -- Niyas
Re: [PATCH] Add native windows on arm64 support
On 12/12/2022 23:56, Michael Paquier wrote:
On Mon, Dec 12, 2022 at 01:38:37PM +, Niyas Sait wrote:
On 05/12/2022 18:14, Andres Freund wrote:
I think the old build system specific part is really minimal in the patch. I
can strip out those if that's preferred.
Removing all the changes from src/tools/msvc/ is an approach that
works for me.
I've attached a new version (v7) that removes changes to support old
MSVC build system.
--
NiyasFrom 50e8affb5bcb3f6a50a223053fc92145a5709e82 Mon Sep 17 00:00:00 2001
From: Niyas Sait
Date: Fri, 16 Dec 2022 10:45:56 +
Subject: [PATCH v7] Enable postgres native build for windows-arm64 platform
- Add support for meson build
- Add arm64 definition of spin_delay function
- Exclude arm_acle.h import for MSVC
---
doc/src/sgml/install-windows.sgml | 3 ++-
meson.build | 33 +++
src/include/storage/s_lock.h | 20 +--
src/port/pg_crc32c_armv8.c| 2 ++
src/tools/msvc/gendef.pl | 8
5 files changed, 46 insertions(+), 20 deletions(-)
diff --git a/doc/src/sgml/install-windows.sgml
b/doc/src/sgml/install-windows.sgml
index bbd4960e7b..3f865d7d3b 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -352,7 +352,8 @@ $ENV{MSBFLAGS}="/m";
Special Considerations for 64-Bit Windows
- PostgreSQL will only build for the x64 architecture on 64-bit Windows.
+ PostgreSQL will only build for the x64 and ARM64 architectures on 64-bit
+ Windows.
diff --git a/meson.build b/meson.build
index 725e10d815..e354ad7650 100644
--- a/meson.build
+++ b/meson.build
@@ -1944,7 +1944,13 @@ int main(void)
elif host_cpu == 'arm' or host_cpu == 'aarch64'
- prog = '''
+ if cc.get_id() == 'msvc'
+cdata.set('USE_ARMV8_CRC32C', false)
+cdata.set('USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK', 1)
+have_optimized_crc = true
+ else
+
+prog = '''
#include
int main(void)
@@ -1960,18 +1966,19 @@ int main(void)
}
'''
- 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
+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
endif
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index 8b19ab160f..94d2380393 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -707,15 +707,31 @@ 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.
+/*
+ * If using Visual C++ on Win64, inline assembly is unavailable.
+ * Use architecture specific intrinsics.
*/
#if defined(_WIN64)
+/*
+ * For Arm64, use __isb intrinsic. See aarch64 inline assembly definition for
details.
+ */
+#ifdef _M_ARM64
+static __forceinline void
+spin_delay(void)
+{
+/* Reference:
https://learn.microsoft.com/en-us/cpp/intrinsics/arm64-intrinsics#BarrierRestrictions
*/
+ __isb(_ARM64_BARRIER_SY);
+}
+#else
+/*
+ * For x64, use _mm_pause intrinsic instead of rep nop.
+ */
static __forceinline void
spin_delay(void)
{
_mm_pause();
}
+#endif
#else
static __forceinline void
spin_delay(void)
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
+#endif
#include "port/pg_crc32c.h"
diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
index d6bed1
Re: [PATCH] Add native windows on arm64 support
Hi,
On 05/12/2022 18:14, Andres Freund wrote:
With meson gaining in maturity, perhaps that's not the most urgent
thing as we will likely remove src/tools/msvc/ soon but I'd rather do
that right anyway as much as I can to avoid an incorrect state in the
tree at any time in its history.
I'd actually argue that we should just not add win32 support to
src/tools/msvc/.
I think the old build system specific part is really minimal in the
patch. I can strip out those if that's preferred.
--- 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
+ /*
+* See spin_delay aarch64 inline assembly definition above for details
+* ref:
https://learn.microsoft.com/en-us/cpp/intrinsics/arm64-intrinsics#BarrierRestrictions
+ */
+ __isb(_ARM64_BARRIER_SY);
+#else
_mm_pause();
+#endif
}
#else
static __forceinline void
This looks somewhat wrong to me. We end up with some ifdefs on the function
defintion level, and some others inside the function body. I think it should
be either or.
Ok, I can add an MSVC/ARM64 specific function.
diff --git a/meson.build b/meson.build
index 725e10d815..e354ad7650 100644
--- a/meson.build
+++ b/meson.build
@@ -1944,7 +1944,13 @@ int main(void)
elif host_cpu == 'arm' or host_cpu == 'aarch64'
- prog = '''
+ if cc.get_id() == 'msvc'
+cdata.set('USE_ARMV8_CRC32C', false)
+cdata.set('USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK', 1)
+have_optimized_crc = true
+ else
+
+prog = '''
#include
int main(void)
Why does this need to be hardcoded? The compiler probe should just work for
msvc.
There are couple of minor issues in the code probe with MSVC such as
arm_acle.h needs to be removed and requires an explicit import of
intrin.h. But even with those fixes, USE_ARMV8_CRC32C would be set and
no runtime CRC extension check will be performed. Since CRC extension is
optional in ARMv8, It would be better to use the CRC variant with
runtime check. So I end up following the x64 variant and hardcoded the
flags in case of ARM64 and MSVC.
--
Niyas
Re: [PATCH] Add native windows on arm64 support
On 05/12/2022 05:12, Michael Paquier wrote:
- Last comes OpenSSL, that supports amd64_arm64 as build target (see
NOTES-WINDOWS.md), and the library names are libssl.lib and
libcrypto.lib. Looking at
https://slproweb.com/products/Win32OpenSSL.html, there are
experimental builds for arm64 with OpenSSL 3.0. Niyas or somebody
else, could you look at the contents of lib/VC/ and see what we could
rely on for the debug builds after installing this MSI? We should
rely on something like lib/VC/sslcrypto64MD.lib or
lib/VC/sslcrypto32MD.lib, but for arm64.
I tried that installer. And I can see following libraries installed in
lib/VC location.
libcryptoarm64MD.lib
libcryptoarm64MDd.lib
libcryptoarm64MT.lib
libcryptoarm64MTd.lib
libsslarm64MD.lib
libsslarm64MDd.lib
libsslarm64MT.lib
libsslarm64MTd.lib
- USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK => undef,
+ USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK => $self->{platform} eq "ARM64" ? :
1 : undef,
Did you actually test this patch? This won't work at all with perl,
per se the double colon after the question mark.
Yes I did a full build. I am not sure why I didn't see any error with
that. My perl skills are very limited and I started with something bit
more naive like "$self->{platform} == "ARM64" ? : 1 : undef" But that
didn't work and I changed to using eq and the compilation was fine.
Thanks for fixing the patch.
> For now, please find attached an updated patch with all the fixes I
> could come up with.
Thanks. I did a quick sanity build with your updated patch and looks fine.
--
Niyas
Re: [PATCH] Add native windows on arm64 support
Hello,
I've attached a new revision of the patch (v5) and includes following
changes,
1. Add support for meson build system
2. Extend MSVC scripts to handle ARM64 platform.
3. Add arm64 definition of spin_delay function.
4. Exclude arm_acle.h import with MSVC compiler.
V4->V5: * Added reference to Microsoft arm64 intrinsic documentation
* Conditionally enable USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK
* Fixed styling issue spotted by Ian Lawerence Barwick
V3->V4: Add support for meson build system
V2->V3: Removed ASLR enablement and rebased on master.
V1->V2: Rebased on top of master
--
NiyasFrom 58b82107088456fc71d239f4e1b995d91a94b4ef Mon Sep 17 00:00:00 2001
From: Niyas Sait
Date: Tue, 22 Feb 2022 13:07:24 +
Subject: [PATCH v5] Enable postgres native build for windows-arm64 platform
Following changes are included
- Extend MSVC scripts to handle ARM64 platform
- Add arm64 definition of spin_delay function
- Exclude arm_acle.h import for MSVC
- Add support for meson build
---
meson.build | 33 +++-
src/include/storage/s_lock.h | 10 +-
src/port/pg_crc32c_armv8.c | 2 ++
src/tools/msvc/MSBuildProject.pm | 16
src/tools/msvc/Mkvcbuild.pm | 9 +++--
src/tools/msvc/Solution.pm | 11 ---
src/tools/msvc/gendef.pl | 8
7 files changed, 62 insertions(+), 27 deletions(-)
diff --git a/meson.build b/meson.build
index 725e10d815..e354ad7650 100644
--- a/meson.build
+++ b/meson.build
@@ -1944,7 +1944,13 @@ int main(void)
elif host_cpu == 'arm' or host_cpu == 'aarch64'
- prog = '''
+ if cc.get_id() == 'msvc'
+cdata.set('USE_ARMV8_CRC32C', false)
+cdata.set('USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK', 1)
+have_optimized_crc = true
+ else
+
+prog = '''
#include
int main(void)
@@ -1960,18 +1966,19 @@ int main(void)
}
'''
- 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
+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
endif
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index 8b19ab160f..ab6a6e0281 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
+ /*
+* See spin_delay aarch64 inline assembly definition above for details
+* ref:
https://learn.microsoft.com/en-us/cpp/intrinsics/arm64-intrinsics#BarrierRestrictions
+ */
+ __isb(_ARM64_BARRIER_SY);
+#else
_mm_pause();
+#endif
}
#else
static __forceinline void
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
+#endif
#include "port/pg_crc32c.h"
diff --git a/src/tools/msvc/MSBuildProject.pm b/src/tools/msvc/MSBuildProject.pm
index 58590fdac2..274ddc8860 100644
--- a/src/tools/msvc/MSBuildProject.pm
+++ b/src/tools/msvc/MSBuildProject.pm
@@ -310,10 +310,18 @@ sub WriteItemDefinitionGroup
: ($self->{type} eq "dll" ? 'DynamicLibrary' : 'StaticLibrary');
my $libs = $self->Ge
Re: [PATCH] Add native windows on arm64 support
On 02/12/2022 05:41, John Naylor wrote:
I couldn't find something more official for the sse2neon library part.
Not quite sure what this is referring to, but it seems we can just point to
the __aarch64__ section in the same file, which uses the same instruction:
spin_delay(void)
{
__asm__ __volatile__(
" isb; \n");
}
...and which already explains the choice with a comment.
Good point. Will add the comment.
+ if cc.get_id() == 'msvc'
+cdata.set('USE_ARMV8_CRC32C', false)
+cdata.set('USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK', 1)
+have_optimized_crc = true
+ else
That seems like a heavy-handed way to force it. Could we just use the same
gating in the test program that the patch puts in the code of interest?
Namely:
+#ifndef _MSC_VER
#include
+#endif
I took a similar approach as x86 MSVC code. I don't think the test
program would work with MSVC. The compiler options are not MSVC friendly.
--
Niyas
Re: [PATCH] Add native windows on arm64 support
On 02/12/2022 05:02, Michael Paquier wrote: Thanks for the updated version. I have been looking at it closely and it looks like it should be able to do the job (no arm64 machine for Windows here, sigh). I have one tiny comment about this part: - USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK => undef, + USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK => 1, Shouldn't we only enable this flag when we are under aarch64? Similarly, I don't think that it is a good idea to switch on USE_SSE42_CRC32C_WITH_RUNTIME_CHECK all the time. We should only set it when building under x86 and x86_64. Ok, I will fix ARM64 specific one in the next revision. -- Niyas
Re: [PATCH] Add native windows on arm64 support
Hello,
I've attached a new revision of the patch (v4) and includes following
changes,
1. Add support for meson build system
2. Extend MSVC scripts to handle ARM64 platform.
3. Add arm64 definition of spin_delay function.
4. Exclude arm_acle.h import with MSVC compiler.
V3->V4: Add support for meson build system
V2->V3: Removed ASLR enablement and rebased on master.
V1->V2: Rebased on top of master
--
NiyasFrom 872f538f6261b36a32cbcecae82e99778b747799 Mon Sep 17 00:00:00 2001
From: Niyas Sait
Date: Tue, 22 Feb 2022 13:07:24 +
Subject: [PATCH v4] Enable postgres native build for windows-arm64 platform
Following changes are included
- Extend MSVC scripts to handle ARM64 platform
- Add arm64 definition of spin_delay function
- Exclude arm_acle.h import for MSVC
- Add support for meson build
---
meson.build | 33 +++-
src/include/storage/s_lock.h | 10 +-
src/port/pg_crc32c_armv8.c | 2 ++
src/tools/msvc/MSBuildProject.pm | 16
src/tools/msvc/Mkvcbuild.pm | 9 +++--
src/tools/msvc/Solution.pm | 11 ---
src/tools/msvc/gendef.pl | 8
7 files changed, 62 insertions(+), 27 deletions(-)
diff --git a/meson.build b/meson.build
index 725e10d815..e354ad7650 100644
--- a/meson.build
+++ b/meson.build
@@ -1944,7 +1944,13 @@ int main(void)
elif host_cpu == 'arm' or host_cpu == 'aarch64'
- prog = '''
+ if cc.get_id() == 'msvc'
+cdata.set('USE_ARMV8_CRC32C', false)
+cdata.set('USE_ARMV8_CRC32C_WITH_RUNTIME_CHECK', 1)
+have_optimized_crc = true
+ else
+
+prog = '''
#include
int main(void)
@@ -1960,18 +1966,19 @@ int main(void)
}
'''
- 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
+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
endif
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
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
+#endif
#include "port/pg_crc32c.h"
diff --git a/src/tools/msvc/MSBuildProject.pm b/src/tools/msvc/MSBuildProject.pm
index 58590fdac2..274ddc8860 100644
--- a/src/tools/msvc/MSBuildProject.pm
+++ b/src/tools/msvc/MSBuildProject.pm
@@ -310,10 +310,18 @@ sub WriteItemDefinitionGroup
: ($self->{type} eq "dll" ? 'DynamicLibrary' : 'StaticLibrary');
my $libs = $self->GetAdditionalLinkerDependencies($cfgname, ';');
- my $targetmachine =
- $self->{platform} eq 'Win32' ? 'MachineX86' : 'MachineX64';
- my $arc
Re: [PATCH] Add native windows on arm64 support
On 07/11/2022 06:58, Michael Paquier wrote:
#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.
Seems so. Hmm, where does _ARM64_BARRIER_SY come from? Perhaps it
would be better to have a comment referring to it from a different
place than the forums of arm, like some actual docs?
_ARM64_BARRIER_SY is defined in Microsoft Arm64 intrinsic documentation
-
https://learn.microsoft.com/en-us/cpp/intrinsics/arm64-intrinsics?view=msvc-170#BarrierRestrictions
I couldn't find something more official for the sse2neon library part.
--
Niyas
Re: [PATCH] Add native windows on arm64 support
On 05/11/2022 18:31, Andres Freund wrote:
On 2022-11-03 11:06:46 +, 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.
Thanks. I will add changes to add meson build support.
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
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.
Yes, will handle that.
--
Niyas
Re: [PATCH] Add native windows on arm64 support
> I've gone ahead and marked it as "Committed", as that appears to have happened > back in August as 36389a060c. > If for whatever reason that was the Wrong Thing To Do, please let me know. 36389a060c commit enables ASLR for windows but does not include other changes required for Arm64. 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. On Thu, 3 Nov 2022 at 08:38, Ian Lawrence Barwick wrote: > 2022年9月1日(木) 13:42 Michael Paquier : > > > > On Wed, Aug 31, 2022 at 10:22:06PM -0400, Tom Lane wrote: > > > Michael Paquier writes: > > > > The input object could also be reworked so as we would not have any > > > > ordering issues, say > "pre&deeppost". > > > > Changing only the path, you could use "/e/n2" instead of "node()", so > > > > as we'd always get the most internal member. > > > > > > Done that way. > > > > Cool, thanks. bowerbird looks happy after its first run. > > > > An argument in favor of backpatching is if one decides to build the > > code with MSVC and patch the scripts to enable ASLR. However, nobody > > has complained about that yet, so fine by me to leave this change only > > on HEAD for now. > > Apologies for the thread bump, but there is an entry for this thread > in the current CF > previously marked "Needs review": > > https://commitfest.postgresql.org/40/3561/ > > I've gone ahead and marked it as "Committed", as that appears to have > happened > back in August as 36389a060c. > > If for whatever reason that was the Wrong Thing To Do, please let me know. > > Regards > > Ian Barwick > -- Niyas Sait Linaro Limited v3-0001-Enable-postgres-native-build-for-windows-arm64-pl.patch Description: Binary data
Re: [PATCH] Add native windows on arm64 support
Hello, Please find a new patch (no further changes) rebased on top of the master. On Tue, 10 May 2022 at 02:02, Michael Paquier wrote: > On Mon, May 09, 2022 at 12:44:22PM +0100, Niyas Sait wrote: > > Microsoft updated documentation [1] and clarified that ASLR cannot be > > disabled for Arm64 targets. > > > > Because ASLR can't be disabled on ARM, ARM64, or ARM64EC architectures, > > > /DYNAMICBASE:NO isn't supported for these targets. > > Better than nothing, I guess, though this does not stand as a reason > explaining why this choice has been made. And it looks like we will > never know. We are going to need more advanced testing to check that > DYNAMICBASE is stable enough if we begin to enable it. Perhaps we > should really look at that for all the build targets we currently > support rather than just ARM, for the most modern Win32 environments > if we are going to cut support for most of the oldest releases.. > -- > Michael > -- Niyas Sait Linaro Limited v2-0001-Enable-postgres-native-build-for-windows-arm64-pl.patch Description: Binary data
Re: [PATCH] Add native windows on arm64 support
> Thanks for doing that! Your post is just a couple of days old, let's > see if we get a reply on that. Microsoft updated documentation [1] and clarified that ASLR cannot be disabled for Arm64 targets. Because ASLR can't be disabled on ARM, ARM64, or ARM64EC architectures, > /DYNAMICBASE:NO isn't supported for these targets. Thanks Niyas [1] https://docs.microsoft.com/en-us/cpp/build/reference/dynamicbase?view=msvc-170 On Mon, 25 Apr 2022 at 02:17, Michael Paquier wrote: > On Thu, Apr 21, 2022 at 10:21:04AM +0100, Niyas Sait wrote: > > The following error occurs: > > > > LINK : fatal error LNK1246: '/DYNAMICBASE:NO' not compatible with 'ARM64' > > target machine; link without '/DYNAMICBASE:NO > > Okay, that's interesting. In light of things like 7f3e17b, that may > be annoying. Perhaps newer Windows versions are able to handle that > better. I am wondering if it would be worth re-evaluating this > change, and attempt to re-enable that in environments other than > arm64. This could become interesting if we consider that there have > been talks to cut the support for a bunch of Windows versions to focus > on the newer ones only. > > > This seems to be a deliberate restriction for Arm64 targets. However, no > > references were provided. To clarify, I have posted a question [1] on the > > community channel of Visual Studio. > > Thanks for doing that! Your post is just a couple of days old, let's > see if we get a reply on that. > -- > Michael >
Re: [PATCH] Add native windows on arm64 support
> Why does it not allow that? Is that just a limitation of the > compiler? If yes, what is the error happening? This question is not > exactly answered yet as of this thread. I may be missing a reference > about that in the upstream docs, but I see nowhere an explanation > about the reason and the why. That's also one of the first questions > from Thomas upthread. The following error occurs: LINK : fatal error LNK1246: '/DYNAMICBASE:NO' not compatible with 'ARM64' target machine; link without '/DYNAMICBASE:NO This seems to be a deliberate restriction for Arm64 targets. However, no references were provided. To clarify, I have posted a question [1] on the community channel of Visual Studio. Niyas [1] https://developercommunity.visualstudio.com/t/LINK-:-fatal-error-LNK1246:-DYNAMICBAS/10020163 On Thu, 21 Apr 2022 at 05:07, Michael Paquier wrote: > On Wed, Apr 20, 2022 at 10:43:06AM +0100, Niyas Sait wrote: > >> This issue is still lying around, and you may have been lucky. Would > >> there be any issues to remove this change to get a basic support in? > >> As mentioned upthread, there is a long history of Postgres with ASLR. > > > > MSVC linker doesn't allow non-random base addresses for Arm64 platforms. > > It is needed for basic support. > > Why does it not allow that? Is that just a limitation of the > compiler? If yes, what is the error happening? This question is not > exactly answered yet as of this thread. I may be missing a reference > about that in the upstream docs, but I see nowhere an explanation > about the reason and the why. That's also one of the first questions > from Thomas upthread. > -- > Michael >
Re: [PATCH] Add native windows on arm64 support
> Have you tested with the amount of coverage provided by vcregress.pl?
I built and ran the relevant tests with the help of run_build.pl.
I think following tests are executed - check, contribcheck, ecpgcheck,
installcheck, isolationcheck, modulescheck, and upgradecheck.
> Another thing I was wondering about is if it would be possible to have
> this option in Travis, but that does not seem to be the case:
> https://docs.travis-ci.com/user/reference/windows/#windows-version
Yes, I think Travis doesn't yet support Windows/Arm64 platform.
> + if ($solution->{platform} eq 'ARM64') {
> + push(@pgportfiles, 'pg_crc32c_armv8_choose.c');
> + push(@pgportfiles, 'pg_crc32c_armv8.c');
> + } else {
> + push(@pgportfiles, 'pg_crc32c_sse42_choose.c');
> + push(@pgportfiles, 'pg_crc32c_sse42.c');
> + }
>
> +++ b/src/port/pg_crc32c_armv8.c
> +#ifndef _MSC_VER
> #include
> +#endif
> [ ... ]
> +#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
> I think that such extra optimizations had better be in a separate
> patch, and we should focus on getting the build done first.
> This would mean that a basic patch could be done with just the changes
> for gendef.pl, and the first part of the changes inMSBuildProject.pm.
> Would that not be enough?
I cannot build without any of the above changes. Nothing specific for
optimization
is added to this patch.
> + # arm64 linker only supports dynamic base address
> + my $cfgrandbaseaddress = $self->{platform} eq 'ARM64' ? 'true' :
'false';
> This issue is still lying around, and you may have been lucky. Would
> there be any issues to remove this change to get a basic support in?
> As mentioned upthread, there is a long history of Postgres with ASLR.
MSVC linker doesn't allow non-random base addresses for Arm64 platforms.
It is needed for basic support.
Niyas
Re: [PATCH] Add native windows on arm64 support
> Niyas, any updates on that? Sorry for the delay! Configuring the scripts took some time. I have successfully run the builfarm animal script using my git repository [1] which contains the proposed patch on a Windows Arm64 machine. I made a request to add a new machine to build farm through [2]. I believe we should be able to enable the build farm machine once the change has been merged. Thanks, Niyas [1] https://github.com/nsait-linaro/postgres.git [2] https://buildfarm.postgresql.org/cgi-bin/register-form.pl On Thu, 7 Apr 2022 at 18:54, Andres Freund wrote: > Hi, > > On 2022-04-07 13:42:49 -0400, Tom Lane wrote: > > Andres Freund writes: > > > On 2022-04-07 09:40:43 -0400, Tom Lane wrote: > > >> If it causes problems down the road, how will we debug it? > > > > > If what causes problems down the road? Afaics the patch doesn't change > > > anything outside of windows-on-arm, so it shouldn't cause any breakage > we care > > > about until we get a buildfarm animal. > > > > Do we have a volunteer to run a buildfarm animal? I don't see much > > point in thinking about this till there is one ready to go. > > OP said that they might be able to: > > https://www.postgresql.org/message-id/CAFPTBD_NZb%3Dq_6uE-QV%2BS%2Bpm%3DRc9sBKrg8CQ_Y3dc27Awrm7cQ%40mail.gmail.com > > Niyas, any updates on that? > > Greetings, > > Andres Freund >
Re: [PATCH] Add native windows on arm64 support
Thanks, Thomas for reviewing the patch!
Yes, we cannot turn off ASLR for Arm64 targets with MSVC. I haven't seen
any issues so far on win/arm64 with this option turned off. I guess it
should be okay. If we really need ASLR turned off, then I guess might have
to use clang.
Yes, we could look into providing a build machine. Do you have any
reference to what the CI system looks like now for PostgresSQL and how to
add new workers etc.?
Niyas
On Fri, 18 Mar 2022 at 20:50, Thomas Munro wrote:
> On Wed, Feb 23, 2022 at 11:09 PM Niyas Sait wrote:
> > I have created a patch that adds support for building Postgres for the
> windows/arm64 platform using the MSVC toolchain. Following changes have
> been included
> >
> > 1. Extend MSVC scripts to handle ARM64 platform.
> > 2. Add arm64 definition of spin_delay function.
> > 3. Exclude arm_acle.h import with MSVC compiler.
> >
> > Compilation steps are consistent and similar to other windows platforms.
> >
> > The change has been tested on windows/x86_64 and windows/arm64 and all
> regression tests passes on both platforms.
>
> +# arm64 linker only supports dynamic base address
> +my $cfgrandbaseaddress = $self->{platform} eq 'ARM64' ? 'true' :
> 'false';
> ...
> + $cfgrandbaseaddress
>
> Does that mean that you can't turn off ASLR on this arch? Does it
> cause random backend failures due to inability to map shared memory at
> the expected address? Our experience with EXEC_BACKEND on various
> Unix systems tells us that you have to turn off ASLR if you want 100%
> success rate on starting backends, but I guess it depends on the
> details of how the randomisation is done.
>
> Any interest in providing a build farm animal, so that we could know
> that this works?
>
[PATCH] Add native windows on arm64 support
Hello, I have created a patch that adds support for building Postgres for the windows/arm64 platform using the MSVC toolchain. Following changes have been included 1. Extend MSVC scripts to handle ARM64 platform. 2. Add arm64 definition of spin_delay function. 3. Exclude arm_acle.h import with MSVC compiler. Compilation steps are consistent and similar to other windows platforms. The change has been tested on windows/x86_64 and windows/arm64 and all regression tests passes on both platforms. Thanks, Niyas enable-native-windows-arm64-build.patch Description: Binary data
