llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.3k stars 11.68k forks source link

_xend() declaration in intrin.h may cause function multiversioning error on Windows #95133

Open stampho opened 3 months ago

stampho commented 3 months ago

This is a simplified example to reproduce the issue:

#include <immintrin.h>

#pragma clang attribute push(__attribute__((target("avx"))), apply_to=function)
#include <atomic>
#pragma clang attribute pop

int main()
{
    return 0;
}

Build the example with: clang-cl.exe /c main.cpp /Fomain.obj /nologo /std:c++20 or clang-cl.exe /c main.cpp /Fomain.obj /nologo -D__RTM__

The result is:

In file included from main.cpp:1:
In file included from C:\tools\LLVM-18.1.6\lib\clang\18\include\immintrin.h:603:
C:\tools\LLVM-18.1.6\lib\clang\18\include\rtmintrin.h(36,1): error: attribute 'target' multiversioning cannot be combined with attribute 'always_inline'
   36 | _xend(void)
      | ^
C:\tools\LLVM-18.1.6\lib\clang\18\include\intrin.h(167,6): note: function multiversioning caused by this declaration
  167 | void _xend(void);
      |      ^
1 error generated.

I reproduce this with:

clang version 18.1.6
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Tools\LLVM-18.1.6\bin

and

Visual Studio Community 2022
17.10.1

The example is artificial, I have the same issue when building skia ( https://github.com/google/skia ) with clang-cl and c++20 enabled. I'm not absolutely sure if this is really a clang issue but it seems reasonable to me to push attributes before including a header which declares a set of functions which also happen to use standard library functions. Does the declaration of _xend() in intrin.h really necessary if the intrinsics are enabled?

wbaby commented 1 month ago

Same problem, is there a solution?

In file included from randombytes/internal/randombytes_internal_random.c:53: In file included from /opt/zcash-win/depends/x86_64-w64-mingw32/native/lib/clang/18/include/immintrin.h:36: /opt/zcash-win/depends/x86_64-w64-mingw32/native/lib/clang/18/include/pmmintrin.h:58:1: error: attribute 'target' multiversioning cannot be combined with attribute 'always_inline' 58 | _mm_addsub_ps(m128 a, m128 b) | ^

vule-levu commented 2 weeks ago

I don't see any error for the OP code, only a warning, warning: unused attribute 'target' in '#pragma clang attribute push' region [-Wpragma-clang-attribute] 5 | #pragma clang attribute push(__attribute__((target("avx"))), apply_to=function) | ^ test.cpp(7,15): note: '#pragma clang attribute push' regions ends here 7 | #pragma clang attribute pop | ^ 1 warning generated.

Which compiler could analyze logics best in this case when you try to inline one specific method while requiring your compiler to create multiple versions of the method to target different CPU's?

Nick-Kooij commented 1 week ago

I've been bitten by the same issue, albeit with LLVM 19.1.0 and Visual Studio 2022 17.11.4.

A workaround is to ignore the <rtmintrin.h> header entirely, by defining its include guard, before including <immintrin.h>.

#define __RTMINTRIN_H
#include <immintrin.h>

Ignoring this header likely shouldn't cause issue, as it appears to be related to hardware transactional memory Restricted Transactional Memory (RTM) instruction set.

It seems very unlikely, if not impossible, that any existing software depends on Transactional Synchronization Extensions instruction set. I believe known hardware implementations have been flawed enough to be removed by subsequent microcode updates by Intel.