On Fri, Dec 2, 2022 at 12:20 AM Niyas Sait <niyas.s...@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