ARM: Clang 17.x crash in libdeflate

[kraj]

Attach test case is a new crash seen with Clang 17.x branch as compared to 16.x where it works ok.

it says
| fatal error: error in backend: Cannot select: intrinsic %llvm.arm.crc32b

and detailed crash log below.

| 1.	<eof> parser at end of file
| 2.	Code generation
| 3.	Running pass 'Function Pass Manager' on module '/mnt/jenkins/workspace/Yocto-world-arm/build/tmp/work/cortexa15t2hf-neon-yoe-linux-gnueabi/libdeflate/1.18/git/lib/crc32.c'.
| 4.	Running pass 'ARM Instruction Selection' on function '@__crc32b'
| Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
| 0  libLLVM-17.so               0x00007f7f23d649b4 llvm::sys::RunSignalHandlers() + 52
| 1  libLLVM-17.so               0x00007f7f23c9a877 llvm::CrashRecoveryContext::HandleExit(int) + 87
| 2  libLLVM-17.so               0x00007f7f23d5f782 llvm::sys::Process::Exit(int, bool) + 34
| 3  arm-yoe-linux-gnueabi-clang 0x00005651d0c708fd
| 4  libLLVM-17.so               0x00007f7f23cac1d3 llvm::report_fatal_error(llvm::Twine const&, bool) + 115
| 5  libLLVM-17.so               0x00007f7f246d9d86 llvm::SelectionDAGISel::CannotYetSelect(llvm::SDNode*) + 422
| 6  libLLVM-17.so               0x00007f7f246ddbef llvm::SelectionDAGISel::SelectCodeCommon(llvm::SDNode*, unsigned char const*, unsigned int) + 10527
| 7  libLLVM-17.so               0x00007f7f26699fe5
| 8  libLLVM-17.so               0x00007f7f246d7b66 llvm::SelectionDAGISel::DoInstructionSelection() + 486
| 9  libLLVM-17.so               0x00007f7f246e245d llvm::SelectionDAGISel::CodeGenAndEmitDAG() + 973
| 10 libLLVM-17.so               0x00007f7f246e49c0 llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) + 3536
| 11 libLLVM-17.so               0x00007f7f246e6456 llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) + 1206
| 12 libLLVM-17.so               0x00007f7f266a33f8
| 13 libLLVM-17.so               0x00007f7f241890c3 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) + 419
| 14 libLLVM-17.so               0x00007f7f23ec086d llvm::FPPassManager::runOnFunction(llvm::Function&) + 829
| 15 libLLVM-17.so               0x00007f7f23ec0a8b llvm::FPPassManager::runOnModule(llvm::Module&) + 59
| 16 libLLVM-17.so               0x00007f7f23ec1209 llvm::legacy::PassManagerImpl::run(llvm::Module&) + 1065
| 17 libclang-cpp.so.17          0x00007f7f2bebb86a clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::StringRef, llvm::Module*, clang::BackendAction, llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>, std::unique_ptr<llvm::raw_pwrite_stream, std::default_delete<llvm::raw_pwrite_stream>>) + 5050
| 18 libclang-cpp.so.17          0x00007f7f2c2999c8
| 19 libclang-cpp.so.17          0x00007f7f2ad880ed clang::ParseAST(clang::Sema&, bool, bool) + 1197
| 20 libclang-cpp.so.17          0x00007f7f2caf83b9 clang::FrontendAction::Execute() + 185
| 21 libclang-cpp.so.17          0x00007f7f2ca85fe5 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 293
| 22 libclang-cpp.so.17          0x00007f7f2cb6ff08 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 920
| 23 arm-yoe-linux-gnueabi-clang 0x00005651d0c7230c cc1_main(llvm::ArrayRef<char const*>, char const*, void*) + 4332
| 24 arm-yoe-linux-gnueabi-clang 0x00005651d0c6de36
| 25 libclang-cpp.so.17          0x00007f7f2c73ee6d
| 26 libLLVM-17.so               0x00007f7f23c9a767 llvm::CrashRecoveryContext::RunSafely(llvm::function_ref<void ()>) + 39
| 27 libclang-cpp.so.17          0x00007f7f2c7414a2 clang::driver::CC1Command::Execute(llvm::ArrayRef<std::optional<llvm::StringRef>>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>*, bool*) const + 322
| 28 libclang-cpp.so.17          0x00007f7f2c706e6e clang::driver::Compilation::ExecuteCommand(clang::driver::Command const&, clang::driver::Command const*&, bool) const + 174
| 29 libclang-cpp.so.17          0x00007f7f2c707631 clang::driver::Compilation::ExecuteJobs(clang::driver::JobList const&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&, bool) const + 129
| 30 libclang-cpp.so.17          0x00007f7f2c71765c clang::driver::Driver::ExecuteCompilation(clang::driver::Compilation&, llvm::SmallVectorImpl<std::pair<int, clang::driver::Command const*>>&) + 332
| 31 arm-yoe-linux-gnueabi-clang 0x00005651d0c6f407 clang_main(int, char**, llvm::ToolContext const&) + 4263
| 32 arm-yoe-linux-gnueabi-clang 0x00005651d0c6a2c3 main + 51
| 33 libc.so.6                   0x00007f7f22a23efb
| 34 libc.so.6                   0x00007f7f22a23fb9 __libc_start_main + 137
| 35 arm-yoe-linux-gnueabi-clang 0x00005651d0c6a305 _start + 37
| arm-yoe-linux-gnueabi-clang: error: clang frontend command failed with exit code 70 (use -v to see invocation)
| clang version 17.0.0 (https://github.com/llvm/llvm-project 45c677d8c62b731df617181e5019316d0b2e1820)
| Target: arm-yoe-linux-gnueabi
| Thread model: posix
| InstalledDir: /mnt/jenkins/workspace/Yocto-world-arm/build/tmp/work/cortexa15t2hf-neon-yoe-linux-gnueabi/libdeflate/1.18/recipe-sysroot-native/usr/bin/arm-yoe-linux-gnueabi

test.zip

[llvmbot]

@llvm/issue-subscribers-backend-arm

[davemgreen]

Thanks for the report. There is reproducer of what I think it is doing here:
https://godbolt.org/z/9bjKMecTr

#define __ARM_FEATURE_CRC32	1
#include <arm_acle.h>

__attribute__((target("armv8-a,crc")))
unsigned  test(unsigned x, unsigned y) {
    return __crc32b(x, y);
}

It seems to define __ARM_FEATURE_CRC in order to get the function definitions from arm_acle.h, and then specifies a target override on the function calling __crc32b. It needs to be marked with "armv8-a,crc" in order to select crc32b as the backend pattern specifies HasV8

llvm-project/llvm/lib/Target/ARM/ARMInstrThumb2.td

Line 3451 in aca8b9d

Requires<[IsThumb2, HasV8, HasCRC]> {

.

This used to inline, but after the changes in d6f994a the __crc32b function is no longer inlined into test. The __crc32b only has v7a+crc, so can't select crc which requires v8a+crc. I'm not sure I agree that disabling the inlining for these force_inline functions was a good idea.

The real root cause is that target attributes are not well supported on Arm, so the code needs to start hacking around it in unsupported ways like defining __ARM_FEATURE_CRC32. The backend patterns needn't really be predicated on HasV8, which would allow the code to compile, but would leave a lot of extra functions that are not inlined, decreasing performance more than need be.

[kraj]

@davemgreen thanks for looking into it. I agree with your suggestion to fix the compilation process for file in question, however I also think that compiler should not crash, that in itself is a bug regardless. Do you see it same way ?

[davemgreen]

@kazutakahirata I think d6f994a has broken this usecase.

You might be able to argue that by overriding __ARM_FEATURE_CRC32 the code is doing something that is unsupported. There still needs to be some way to handle what it is trying to do though.

There is a disconnect between what the frontend checks for crc intrinsics (CRC is available), and what the backend checks (both crc and v8a). The backend needn't be checking both I think. We can allow v7a+crc as a usability improvement. See #65591

That would leave function that are no longer inlined though, which would have a pretty large impact on performance for intrinsics that are designed to always be inlined. The code in libdeflate may be best changed to __attribute__((target("crc")) with newer version of clang, to allow that inlining. I'm not sure I agree with d6f994a that always-inline functions shouldn't be inlined with mismatching target features under Arm.

[kazutakahirata]

IIUC, the caller test has __attribute__((target("armv8-a,crc"))) while the callee __crc32b requires v7a+crc. Since v8a is a superset of v7a, we should allow inlining of the callee. Did I get this right?

If so, could we implement areInlineCompatible for ARM to indicate that it's OK to inline a v7a callee into a v8a caller? The default implementation in llvm/include/llvm/Analysis/TargetTransformInfoImpl.h looks for an exact match of target attributes, which is probably overly restrictive:

  bool areInlineCompatible(const Function *Caller,
                           const Function *Callee) const {
    return (Caller->getFnAttribute("target-cpu") ==
            Callee->getFnAttribute("target-cpu")) &&
           (Caller->getFnAttribute("target-features") ==
            Callee->getFnAttribute("target-features"));
  }

I'm happy to come up with a patch if you are OK with this direction. Of course, I am open to other routes.

[davemgreen]

That sounds like it would help, but I feel it is quite complex for Arm to come up with a set of rules that would fix all cases. For example a v8a function should in general not be inlined into a v7a callee, unless it is marked always_inline in which case it should be. All the functions in arm_acle.h and arm_neon.h were written in the expectation that they would really always be inlined and optimized away. I feel like the logic in the inliner should reflect that and always inline with a always_inline attribute.

There is a fix for the crc part in 65591.