llvm / llvm-project

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

clang-cl and Windows headers define conflicting versions of _m_prefetchw #87515

Open davidben opened 3 months ago

davidben commented 3 months ago

We got a report in BoringSSL of a conflict between some Windows headers and clang-cl: https://bugs.chromium.org/p/boringssl/issues/detail?id=717

In short, if you build the following with MSVC's STL, clang-cl, and C++20...

#define WIN32_LEAN_AND_MEAN
#include <windows.h>

extern "C++" {  // Yes this is odd, see below for why
#include <memory>
}

int main() {}

The following error comes up:

c:\src\header-conflict>cmake -GNinja -B build -DCMAKE_C_COMPILER="c:/Program Files/Microsoft Visual Studio/2022/Professional/VC/Tools/Llvm/x64/bin/clang-cl.exe" -DCMAKE_CXX_COMPILER="c:/Program Files/Microsoft Visual Studio/2022/Professional/VC/Tools/Llvm/x64/bin/clang-cl.exe"
-- The C compiler identification is Clang 17.0.3 with MSVC-like command-line
-- The CXX compiler identification is Clang 17.0.3 with MSVC-like command-line
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: c:/Program Files/Microsoft Visual Studio/2022/Professional/VC/Tools/Llvm/x64/bin/clang-cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: c:/Program Files/Microsoft Visual Studio/2022/Professional/VC/Tools/Llvm/x64/bin/clang-cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done (3.8s)
-- Generating done (0.0s)
-- Build files have been written to: C:/src/header-conflict/build

c:\src\header-conflict>ninja -C build
ninja: Entering directory `build'
[1/2] Building CXX object CMakeFiles\test.dir\test.cc.obj
FAILED: CMakeFiles/test.dir/test.cc.obj
c:\PROGRA~1\MICROS~3\2022\PROFES~1\VC\Tools\Llvm\x64\bin\clang-cl.exe  /nologo -TP   /DWIN32 /D_WINDOWS /GR /EHsc /Zi /Ob0 /Od /RTC1 -std:c++20 -MDd /showIncludes /FoCMakeFiles\test.dir\test.cc.obj /FdCMakeFiles\test.dir\ -c -- C:\src\header-conflict\test.cc
In file included from C:\src\header-conflict\test.cc:5:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.39.33519\include\memory:14:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.39.33519\include\xmemory:12:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.39.33519\include\limits:15:
In file included from c:\PROGRA~1\MICROS~3\2022\PROFES~1\VC\Tools\Llvm\x64\lib\clang\17\include\intrin.h:20:
In file included from c:\PROGRA~1\MICROS~3\2022\PROFES~1\VC\Tools\Llvm\x64\lib\clang\17\include\x86intrin.h:19:
In file included from c:\PROGRA~1\MICROS~3\2022\PROFES~1\VC\Tools\Llvm\x64\lib\clang\17\include\mm3dnow.h:14:
c:\PROGRA~1\MICROS~3\2022\PROFES~1\VC\Tools\Llvm\x64\lib\clang\17\include\prfchwintrin.h(50,1): error: declaration of '_m_prefetchw' has a different language linkage
   50 | _m_prefetchw(volatile const void *__P)
      | ^
C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um\winnt.h(3541,1): note: previous declaration is here
 3541 | _m_prefetchw (
      | ^
1 error generated.
ninja: build stopped: subcommand failed.

(This is MSVC's clang-cl, but the reporter also can repro on Clang 18.)

The significance of C++20 mode seems to be that, in older C++ versions, <windows.h> does not pull in the Windows copy of the intrinsic. I have not determined why yet. The underlying conflict is language-independent, which is what Windows declares _m_prefetchw as:

#ifdef __cplusplus
extern "C" {
#endif
...
VOID
_m_prefetchw (
    _In_ volatile CONST VOID *Source
    );
...
#pragma intrinsic(_m_prefetchw)

Whereas Clang declares it this way, with no extern block:

static __inline__ void __attribute__((__always_inline__, __nodebug__))
_m_prefetchw(volatile const void *__P)
{
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wcast-qual"
  __builtin_prefetch ((const void*)__P, 1, 3 /* _MM_HINT_T0 */);
#pragma clang diagnostic pop
}

BoringSSL needs to wrap the C++ bits in its headers in extern "C++" to undo other projects including our headers inside extern "C" blocks, which was too common to clear out. (People don't expect <openssl/foo.h> to include C++ code.) C++ headers (such as the MSVC STL!) do not react well to being included under extern "C". So we have to use extern "C++" to restore the default language linkage. However, it seems that restoring the default language linkage explicitly is not quite the same as leaving it undecorated:

https://godbolt.org/z/8r8Mzhbvr https://godbolt.org/z/aKoGzMof1

Bizarrely, even the static mismatch seems it would be fatal, yet it isn't here. Does clang-cl special-case this somehow? https://godbolt.org/z/T84eq1j6q

I'm not sure what the right fix is, but given clang-cl is meant to coexist with Windows headers, I suspect the fix should be on the Clang side, one way or another. I'm not sure whether that means to give its intrinsics C linkage, add more quirks to this mismatch allowance, defer defining the intrinsics to Windows headers, or something else altogether.

EugeneZelenko commented 3 months ago

Could you please try 18 or main branch?

rnk commented 3 months ago

We declare other non-static intrinsics as extern "C": https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/intrin.h#L48

Although these are really a bunch of fake declarations.

New versions of clang might not have the issue due to the new intrin0.h change.

tongyuantongyu commented 3 months ago

Hi, original reporter here. I tested that this issue still presents in Clang 18.1.2, and is fixed in main.

May I ask if I can expect this fix to be landed on a future 18.x release, or I should wait until 19?

Neywiny commented 3 months ago

Can confirm the new 18.1.4 does not fix this. I am struggling to compile the main branch, but for now I've wrapped _m_prefetchw in an extern C. Interestingly I'm running into this and some other issues with llvm when I'm trying to recompile tensorflow for my machine (to improve AVX support as it reports there's performance left on the table) on Windows. Tensorflow says 17.0.6 worked. Visual Studio has 17.0.3 in its installer. I may give those a try if this hack doesn't work.

It's interesting you mention the problem is fixed on main because I didn't see any changes to the file that I thought would help. Maybe it's something higher up the include path that I'm not seeing. Did you compile main and have success with this?

tongyuantongyu commented 3 months ago

Did you compile main and have success with this?

Yes. I also hit some weird error when using MSVC, but using Clang provided by Visual Studio I successfully compiled main branch quite smoothly.