[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