[clang] [Clang] Bring initFeatureMap back to AArch64TargetInfo. (PR #96832)

weiwei chen via cfe-commits cfe-commits at lists.llvm.org
Sun Jun 30 20:33:54 PDT 2024


weiweichen wrote:

Here is a  self contained simple repro test

cpp file:
```
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/TargetInfo.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/TargetParser/Host.h"

using namespace llvm;

// Intercept diagnostics from Clang and then bundle them up in an `Error` if
// something bad happens.
struct DiagInterceptor : public clang::DiagnosticConsumer {
  void HandleDiagnostic(clang::DiagnosticsEngine::Level level,
                        const clang::Diagnostic &info) override {
    if (level >= clang::DiagnosticsEngine::Level::Error) {
      // Keep the last message.
      msg.clear();
      info.FormatDiagnostic(msg);
    }
  }

  SmallString<64> msg;
};

int main() {
      // Instantiate the Clang diagnostic engine. Pass in our interceptor.
  clang::IntrusiveRefCntPtr<clang::DiagnosticIDs> ids(
      new clang::DiagnosticIDs());
  clang::IntrusiveRefCntPtr<clang::DiagnosticOptions> diagOpts(
      new clang::DiagnosticOptions());
  DiagInterceptor interceptor;
  clang::DiagnosticsEngine diags(std::move(ids), std::move(diagOpts),
                                 &interceptor, /*ShouldOwnClient=*/false);

  std::string triple = llvm::sys::getDefaultTargetTriple();
  std::string cpu(llvm::sys::getHostCPUName());

  auto opts = std::make_shared<clang::TargetOptions>();
  opts->Triple = triple;
  opts->CPU = cpu;

  // Ask Clang to create the target info for the architecture and CPU. This will
  // populate `opts` with the full target triple and feature set.
  auto targetInfo = std::unique_ptr<clang::TargetInfo>(
      clang::TargetInfo::CreateTargetInfo(diags, opts));

  llvm::dbgs() << cpu << "\n";
  llvm::dbgs() << triple << "\n";
  
  for (StringRef feature : opts->Features) {
    llvm::dbgs() << feature << "\n";
  }

  return 0;
}
```

CMakeLists.txt
```
cmake_minimum_required(VERSION 3.20.0)
project(TestClangAPI)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED YES)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-exceptions -fno-rtti")

find_package(LLVM REQUIRED CONFIG)
find_package(Clang REQUIRED CONFIG)

message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}")
message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")

# Set your project compile flags.
# E.g. if using the C++ header files
# you will need to enable C++11 support
# for your compiler.

include_directories(${LLVM_INCLUDE_DIRS})
include_directories(${CLANG_INCLUDE_DIRS})
message(STATUS "llvm include dires: ${LLVM_INCLUDE_DIRS}")
message(STATUS "clang include dires: ${CLANG_INCLUDE_DIRS}")
separate_arguments(LLVM_DEFINITIONS_LIST NATIVE_COMMAND ${LLVM_DEFINITIONS})
add_definitions(${LLVM_DEFINITIONS_LIST})

# Now build our tools
add_executable(test test.cpp)


# Link against LLVM libraries
target_link_libraries(test LLVMTargetParser LLVMTarget LLVMSupport clangBasic)
```

To build the test (given that LLVM and Clang can be found correctly):
```
mkdir -p build
cd build
cmake -G Ninja .. --fresh
ninja test
./test
```

Before [this change](https://github.com/llvm/llvm-project/commit/a03d06a736fd8921c8f40637667d6af9c2c76505#diff-2ccae12096c75c4b8422ea0d2fdf6b195896d2554d62cce604e8fcb56a78ef62L1067), we get (non empty features):
```
neoverse-v1
aarch64-unknown-linux-gnu
+aes
+bf16
+complxnum
+crc
+dotprod
+fp-armv8
+fp16fml
+fullfp16
+i8mm
+jsconv
+lse
+neon
+pauth
+rand
+ras
+rcpc
+rdm
+sha2
+sha3
+sm4
+spe
+ssbs
+sve
```

With current top of upstream, we get (empty features):
```
neoverse-v1
aarch64-unknown-linux-gnu
```

And the same test can be run an x86 machine, and we get (non empty features with top of upstream):
```
skylake-avx512
x86_64-unknown-linux-gnu
+adx
+aes
+avx
+avx2
+avx512bw
+avx512cd
+avx512dq
+avx512f
+avx512vl
+bmi
+bmi2
+clflushopt
+clwb
+cmov
+crc32
+cx16
+cx8
+evex512
+f16c
+fma
+fsgsbase
+fxsr
+invpcid
+lzcnt
+mmx
+movbe
+pclmul
+pku
+popcnt
+prfchw
+rdrnd
+rdseed
+sahf
+sse
+sse2
+sse3
+sse4.1
+sse4.2
+ssse3
+x87
+xsave
+xsavec
+xsaveopt
```

This looks like the upstream change, while working good for Clang internally, it has broken the public clang API `clang::TargetInfo::CreateTargetInfo` which makes it inconsistent between different backends 😢 .


As a downstream consumer of upstream, we are broken due to the change. However, we are able to workaround it by adding a wrapper to pull the old logic back for AArch64 backend on our side, but it probably would be much better if upstream can be fixed for `AArch64`.

Please let me know if I can provide more info on reproduce the issue. 

https://github.com/llvm/llvm-project/pull/96832


More information about the cfe-commits mailing list