Fix compiler-rt build with aarch64 using -march=armv8-a

[raj.khem]

Summary

We have a problem when building compiler-rt for aarch64 with 5.0 branch based clang. The issue is while compiling compiler-rt
lib/scudo/scudo_crc32.cpp

#if defined(SSE4_2) || defined(__ARM_FEATURE_CRC32)
u32 computeHardwareCRC32(u32 Crc, uptr Data) {

return CRC32_INTRINSIC(Crc, Data);

}

here We see that when using -march=armv8-a on clang cmdline __ARM_FEATURE_CRC32 gets defined which I doubt is right it should
only get defined when -march=armv8-a+crc or -march=armv8.1-a, what we see in compiler-rt cmake system groks for crc support and if
it sees the support it add -mcrc to cmdline which seems correct howvever clang does not translate this correctly to assembler cmdline

and it endup with errors like

/tmp/tmp.TPh21LESIa/scudo_crc32-b4bfcd.s: Assembler messages:
/tmp/tmp.TPh21LESIa/scudo_crc32-b4bfcd.s:14: Error: selected processor does not support `crc32cx w0,w0,x1'

This change work arounds the clang drivers issues by replacing -mcrc with -march=armv8.1-a which has the same effect but
helps the clang driver to call assembler with right options.

[peter.smith]

While this may make the build work I'm not sure that this is the right long term fix. In particular the is COMPILER_RT_HAS_MCRC_FLAG is specifically tied to the -mcrc flag, I don't think that it should imply armv8.1 as this has more side-effects than just enabling the crc instructions. If clang is doing the wrong thing with regard to the -mcrc flag and __ARM_FEATURE_CRC we should fix clang/llvm rather than work around it here.

When -mcrc went in it https://reviews.llvm.org/D2037 it looks like __ARM_FEATURE_CRC32 was only set when -march=armv8-a+crc (this was pre 8.1). This may have been broken by some later patch though.

Apologies for the drive by comment, I'm on vacation till the 25th July, I will investigate when I get back if you don't get any other comments.

[raj.khem]

I agree its a clang driver problem and I did mention that in my earlier comment, this is a workaround that helps until clang is fixed and its limited to this one file.

[peter.smith]

Regardless of the problems with clang I think having the compiler accept -mcrc imply armv8.1-a is not an acceptable workaround as it could lead to v8.1 only extensions being used by mistake.

Looking into this a little bit I've found that:

• The compiler-rt cmake file test for -mcrc will pass on all aarch64 targets, so at present the crc extensions are always used. There is to the best of my knowledge only one implementation of v8-a (first Applied Micro xgene) that doesn't support the CRC extensions but in ideal world we should still support that option.
• The -mcrc option just adds +crc to the architecture so -march=armv8-a -mcrc is equivalent to -march armv8-a+crc.
• The __ARM_FEATURE_CRC32 macro is only defined when +crc is in the architecture. It is therefore not defined for -march=armv8.1-a. So as far as I can tell the suggested patch will have the effect of disabling the use of crc

So as far as I can see there is at least one problem in clang, the __ARM_FEATURE_CRC32 macro should be defined for -march=armv8.1a. There is a problem (albeit only for the xgene) with the compiler-rt build always using crc extensions for v8-a although I'm not sure what the best fix for that would be right now. It may be better to say something like "if march=armv8-a and no use of -mno_crc, then use -march=armv8-a+crc", in other words if using armv8-a and crc hasn't been disabled then set the arch to -march=armv8-a+crc, although I don't know how to express that in cmake.

In addition I'm a bit confused about is the error message you are seeing, it looks like it comes from the GNU assembler and not the clang integrated assembler. Can you let us know what options you are using to build compiler-rt?

[raj.khem]

I agree clang should not emit __ARM_FEATURE_CRC32 when -march is armv8-a it can generate it if we have march set to armv8-a+crc, there is another issue with compiler-rt build when clang is configured for multiple backends, COMPILER_RT_HAS_MCRC_FLAG macros gets defined even for x86_64 so we actually need to check for target as well.
something like below

if (COMPILER_RT_HAS_MCRC_FLAG)
 if ("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "aarch64")
   set_source_files_properties(scudo_crc32.cpp PROPERTIES COMPILE_FLAGS -march=armv8.1-a)
 endif()
endif()

[raj.khem]

This comment was removed by raj.khem.

[cryptoad]

I fixed at some point COMPILER_RT_HAS_MCRC_FLAG being defined for x86_64, it was due to warnings being disabled, making all the cmake checks pass.
See https://reviews.llvm.org/D46079. It might be what's happening here.

(Edit: didn't see it was an old thread).

[raj.khem]

this is because warning were disabled during cmake tests

lib/scudo/CMakeLists.txt

 # Enable the AArch64 CRC32 feature for scudo_crc32.cpp, if available.
 # Note that it is enabled by default starting with armv8.1-a.
 if (COMPILER_RT_HAS_MCRC_FLAG)
-  set_source_files_properties(scudo_crc32.cpp PROPERTIES COMPILE_FLAGS -mcrc)
+  set_source_files_properties(scudo_crc32.cpp PROPERTIES COMPILE_FLAGS -march=armv8.1-a)
 endif()
 
 if(COMPILER_RT_HAS_SCUDO)