MDEV-39692 innodb intrinsics fail to compile on ARMv8.3-A and later (…

[grooverdan]

…but not ARMv9.x)

fails with:

Assembler messages:
{standard input}:169: Error: selected processor does not support retaa' {standard input}:271: Error: selected processor does not support retaa'

It happens because the pmem_cvap() funciton manually inserts the .arch armv8-2.a clause, making GAS believe that retaa instruction (inserted by GCC) is invalid. Bump the manually inserted clause to armv8.3-a, to prevent GAS from choking on the retaa instructions.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@oss.qualcomm.com

Ported from https://lore.kernel.org/openembedded-devel/20260520113418.2523208-1-dmitry.baryshkov@oss.qualcomm.com/ by Daniel Black.

from #3677

[CLAassistant]

All committers have signed the CLA.

[gemini-code-assist]

Code Review

This pull request updates the ARM architecture directive from armv8.2-a to armv8.3-a for the dc cvap instruction in storage/innobase/sync/cache.cc. The reviewer recommends using a generic sys instruction instead of the .arch directive to avoid overriding the compiler's architecture settings, which can lead to assembly errors in other parts of the translation unit.

[gemini-code-assist]

Using the .arch directive in inline assembly is problematic because it overrides the compiler's architecture settings for the remainder of the translation unit. This can cause the assembler to reject instructions from newer architecture versions that the compiler might generate (such as retaa from ARMv8.3-A). While updating to armv8.3-a fixes the reported issue, it remains fragile. A more robust approach is to use the generic sys instruction (sys #3, c7, c12, 1), which encodes dc cvap without requiring a .arch directive, thus avoiding interference with the compiler's architecture state. Note that the same logic applies to the ARMv9 branch on line 86.

    __asm__ __volatile__("sys #3, c7, c12, 1, %0" :: "r"(u) : "memory");

[grooverdan]

seem an equivalent.

godbolt trying to produce the original report of retaa output as yet unsuccessful.

[dr-m]

In your Godbolt link the assembler and disassembler are only enabled for one of the outputs. One can’t really trust the __asm__ output from GCC before it’s actually fed to an assembler. Unlike GCC, Clang has a built-in assembler and does validate the code.

For a proper comparison, I enabled "Compile to binary object" in the output settings and copied each output to a local file so that I could compare them. They are identical with each other. The difference in the source code is as follows:

@@ -17,11 +17,7 @@
   for (uintptr_t u= uintptr_t(buf) & ~(CPU_LEVEL1_DCACHE_LINESIZE),
          end= uintptr_t(buf) + size;
        u < end; u+= CPU_LEVEL1_DCACHE_LINESIZE)
-#if defined __ARM_ARCH && __ARM_ARCH == 9
-    __asm__ __volatile__(".arch armv9.4-a\n dc cvap, %0" :: "r"(u) : "memory");
-#else
-    __asm__ __volatile__(".arch armv8.3-a\n dc cvap, %0" :: "r"(u) : "memory");
-#endif
+      __asm__ __volatile__("sys #3, c7, c12, 1, %0" :: "r"(u) : "memory");
 
   __asm__ __volatile__("dmb ishst" ::: "memory");
 }

We should keep in mind that the outcome may depend on the version of the assembler.

I might expect an old gas to be more likely to support the sys variant rather than some .arch directive. But this would be best tested with some old gas versions. Luckily, users of old build tool chains can work around this by specifying a compile-time configuration parameter:

cmake -DWITH_INNODB_PMEM=OFF

I’d suggest to write this as follows:

void pmem_cvap(const void* buf, size_t size)
{
  for (uintptr_t u= uintptr_t(buf) & ~(CPU_LEVEL1_DCACHE_LINESIZE),
         end= uintptr_t(buf) + size;
       u < end; u+= CPU_LEVEL1_DCACHE_LINESIZE)
      __asm__ __volatile__(/* dc cvap, %0 */ "sys #3, c7, c12, 1, %0" ::
                           "r"(u) : "memory");

[dr-m]

Oh, the retaa might depend on the compiler version and build options. I think that the sys instruction is the best way to go. It would be nice if you could try to find some old gas versions and check how they would translate it.

Also, the sys variant needs to be tested on godbolt.org with a few versions of Clang. That is, enable "Compile to binary object" and check that the same encoding is being emitted in the disassembly.