Re: [PATCH] Add native windows on arm64 support

On Fri, Dec 2, 2022 at 12:20 AM Niyas Sait <niyas.sait@linaro.org> wrote:
>
>
> On 07/11/2022 06:58, Michael Paquier wrote:
> > 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

In particular, at the bottom of that section:

"For the __isb intrinsic, the only restriction that is currently valid is
_ARM64_BARRIER_SY; all other values are reserved by the architecture."

This corresponds to

https://developer.arm.com/documentation/ddi0596/2021-06/Base-Instructions/ISB--Instruction-Synchronization-Barrier-

which says

"SY Full system barrier operation, encoded as CRm = 0b1111. Can be omitted."

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

About v4:

+ * Use _mm_pause (x64) or __isb(arm64) intrinsic instead of rep nop.

Need a space here after __isb.

+  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 <arm_acle.h>
+#endif

--
John Naylor
EDB: http://www.enterprisedb.com