Install clang-scan-deps tool #412

nolange · 2024-03-13T00:22:23Z

Should be pretty safe changes.
(did not want to make a separate PR for the macro update)

mstorsjo · 2024-03-13T09:33:44Z

Should be pretty safe changes. (did not want to make a separate PR for the macro update)

Please don't mix unrelated changes in the same PR. Even if it seems safe, they can have entirely separate issues, warranting separate discussions with different conclusions.

Re using a non-deprecated define for UWP - mingw-w64 headers don't have a single mention of WINAPI_FAMILY_PC_APP, so I don't think that works there. (The whole machinery for api families in mingw-w64 headers is a bit old and hasn't kept up with all the relevant changes in WinSDK - many of the bits in how it is implemented match the first iteration of this from WinSDK 8.0. A later WinSDK update refactored the internals for how it works, which IIRC the mingw-w64 implementation hasn't kept up with that.)

This shows up in the test log, but apparently my smoke test suite didn't catch it properly. See https://github.com/mstorsjo/llvm-mingw/actions/runs/8257442431/job/22594726733?pr=412. The uwp-error testcase, which usually fails like this, https://github.com/mstorsjo/llvm-mingw/actions/runs/8257612173/job/22595195864:

i686-w64-mingw32uwp-clang ../uwp-error.c -o .tested.build.uwp-error-mingw32uwp.exe -Wimplicit-function-declaration -Werror
../uwp-error.c:23:16: error: call to undeclared function 'GetOEMCP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
   23 |     UINT res = GetOEMCP();
      |                ^
1 error generated.
OK: UWP build failed intentionally

Now failed like this:

In file included from ../uwp-error.c:19:
In file included from D:/a/_temp/msys64/llvm-mingw/include/windows.h:69:
In file included from D:/a/_temp/msys64/llvm-mingw/include/windef.h:21:
D:/a/_temp/msys64/llvm-mingw/include/winnt.h:308:11: error: unknown type name 'CONST'
  308 |   typedef CONST WCHAR *LPCWCH,*PCWCH;
      |           ^
[...]
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
OK: UWP build failed intentionally

The smoke testsuite does have a test for compiling an executable in UWP mode, which is supposed to work, but for that I only compiled the plain hello.c which doesn't include windows.h in any form, so that one does succeed. I'll amend it to try to building another test executable that does include and use some Windows APIs in UWP mode as well. I'm running a test to confirm that it would catch this issue sooner, see master...uwp-define.

mstorsjo · 2024-03-13T13:00:57Z

Also, re the libcxx modules, that change in itself looks ok, but if we're going to do that, we'll probably need to do a bit more.

I tried playing with a little, but will need to look further into it later.

The libc++ module files get installed into <root>/<arch>-w64-mingw32/share/libc++/v1 - but we could also want to install them into e.g. <root>/share/libc++/v1 instead. That's easy to achieve by adding -DLIBCXX_INSTALL_MODULES_DIR="$PREFIX/share/libc++/v1" to the build script. But <roo>/<arch>-w64-mingw32/lib/libc++.modules.json contains a relative path to this directory, and that breaks (it gets an incorrect relative path) if we specify LIBCXX_INSTALL_MODULES_DIR as an absolute path. (This should be easy to fix in libcxx though.)

If we're going to use modules, I presume we should ship clang-scan-deps as well (as AFAIK tools like cmake will need to use that), so we should filter it out in strip-llvm.sh. (I guess that goes for C++20 modules overall, not specifically libc++ as a module.)

I'm looking at https://discourse.llvm.org/t/llvm-discussion-forums-libc-c-23-module-installation-support/77087/16 as standalone example of how to use the libcxx modules. (AFAIK CMake integration to use it doesn't quite exist yet.)

I'm also looking at https://www.kitware.com/import-cmake-the-experiment-is-over/ as example for using C++20 modules in general.

nolange · 2024-03-13T14:26:50Z

I will lower my head in shame.

Yes, I somehow missed that mingw has vastly different headers, I previously was sure I checked both the WindowsSDK aswell as MingW.

The modules seem more complex aswell, with clang-scan-deps being necessary.

I did read the CMake Article and planned to play with that, did appear to me that its "solved" with CMake.
In case you need help with that, I added a toolchainfile.
Its intended to be placed in share/llvm-x86_64-w64-mingw32_toolchainfile.cmake in the root of llvm-mingw toolchain,
this hopefully should be enough to direct CMake to the right paths.

if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.20")
  set(_prefix "${CMAKE_CURRENT_LIST_DIR}/../")
  cmake_path(ABSOLUTE_PATH _prefix NORMALIZE)
  set(_filename "${CMAKE_CURRENT_LIST_FILE}")
  cmake_path(GET _filename FILENAME _filename)
else()
  get_filename_component(_prefix "${CMAKE_CURRENT_LIST_DIR}/../" ABSOLUTE)
  get_filename_component(_filename "${CMAKE_CURRENT_LIST_FILE}" NAME)
endif()

string(REGEX REPLACE "^llvm[_-]\(.*\)[_-]toolchain.*" "\\1" _arch "${_filename}")
string(REGEX REPLACE "^.*-mingw32\(.*\)" "\\1" _mingwtype "${_arch}")
string(REGEX REPLACE "-.*" "" _arch "${_arch}")
unset(_filename)

# set(_prefix /opt/llvm-mingw/)
# set(_mingwtype uwp)

set(CMAKE_SYSTEM_NAME Windows)
if(_arch)
  set(CMAKE_SYSTEM_PROCESSOR ${_arch})
else()
  set(CMAKE_SYSTEM_PROCESSOR ${CMAKE_HOST_SYSTEM_PROCESSOR})
endif()

set(CMAKE_ASM_COMPILER "${_prefix}bin/clang")
set(CMAKE_C_COMPILER "${_prefix}bin/clang")
set(CMAKE_CXX_COMPILER "${_prefix}bin/clang++")
set(CMAKE_RC_COMPILER ${_prefix}bin/llvm-rc)

set(CMAKE_CXX_FLAGS_INIT "-stdlib=libc++")

foreach(_lang ASM C CXX)
  set(CMAKE_${_lang}_COMPILER_TARGET ${CMAKE_SYSTEM_PROCESSOR}-w64-mingw32${_mingwtype})
  if(_mingwtype STREQUAL "uwp")
    set(CMAKE_${_lang}_FLAGS_INIT "${CMAKE_${_lang}_FLAGS_INIT} -D_WIN32_WINNT=0x0A00 -DWINVER=0x0A00 -DWINAPI_FAMILY=WINAPI_FAMILY_APP -DUNICODE -D_UCRT")
  endif()
endforeach()
unset(_lang)

set(_linker lld)
# set(_linker_extra " -Wl,--undefined-version")
if(_mingwtype STREQUAL "uwp")
  set(_linker_extra "${_linker_extra} -Wl,-lwindowsapp -Wl,-lucrtapp")
endif()
set(CMAKE_EXE_LINKER_FLAGS_INIT "--start-no-unused-arguments -stdlib=libc++ -fuse-ld=${_linker} -rtlib=compiler-rt -unwindlib=libunwind${_linker_extra} --end-no-unused-arguments")
set(CMAKE_MODULE_LINKER_FLAGS_INIT "${CMAKE_EXE_LINKER_FLAGS_INIT}")
set(CMAKE_SHARED_LINKER_FLAGS_INIT "${CMAKE_EXE_LINKER_FLAGS_INIT}")
set(CMAKE_LINKER ${_prefix}bin/ld.${_linker})
unset(_linker_extra)
unset(_linker)

set(CMAKE_SYSROOT "${_prefix}${CMAKE_SYSTEM_PROCESSOR}-w64-mingw32")

unset(_mingwtype)
unset(_prefix)

set(ENV{PKG_CONFIG_SYSROOT_DIR} "${CMAKE_SYSROOT}")
if($ENV{PKG_CONFIG_LIBDIR})
	set(ENV{PKG_CONFIG_LIBDIR} "${CMAKE_SYSROOT}/usr/lib/pkgconfig:${CMAKE_SYSROOT}/usr/share/pkgconfig:$ENV{PKG_CONFIG_LIBDIR}")
else()
	set(ENV{PKG_CONFIG_LIBDIR} "${CMAKE_SYSROOT}/usr/lib/pkgconfig:${CMAKE_SYSROOT}/usr/share/pkgconfig")
endif()

set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)

nolange · 2024-03-13T14:40:46Z

Well.... guess not.

No builtin support for import std; or other compiler-provided modules.

https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html#manual:cmake-cxxmodules(7)

mstorsjo · 2024-03-13T16:14:30Z

Well.... guess not.

No builtin support for import std; or other compiler-provided modules.

https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html#manual:cmake-cxxmodules(7)

Yep, exactly. So until this is available in some form so we can test it, I think it's best to hold off of merging anything to master, before we've made sure that whatever layout we choose actually works with cmake somewhat out of the box. But until then we can certainly prepare things in a branch.

General C++20 modules with one's own code, as in that linked CMake article, should kinda work though - the only thing it requires probably is that we'd keep the clang-scan-deps executable, and we could probably merge that without waiting for the rest.

build-llvm.sh

Compilers and buildsystems have early support for c++ modules, see [1], [2]. `clang-scan-dep` is necessary for integration in CMake, potentially other buildsystems in the future. Using standard modules like from libc++ is still not solved thoroughly, and module support from libc++ is marked experimental, documentation how to use them is WIP [3]. [1]: https://www.kitware.com/import-cmake-the-experiment-is-over [2]: https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html [3]: llvm/llvm-project#80601

mstorsjo · 2024-04-17T08:39:01Z

Well.... guess not.

No builtin support for import std; or other compiler-provided modules.

https://cmake.org/cmake/help/latest/manual/cmake-cxxmodules.7.html#manual:cmake-cxxmodules(7)

@huangqinjin noted in mstorsjo/msvc-wine#126 that the upcoming CMake 3.30 should have support for this.

I pushed a branch https://github.com/mstorsjo/llvm-mingw/commits/libcxx-modules which enables installing the libc++ module, and installing it into a directory that allows sharing the files across the architectures. (Since llvm/llvm-project@0cd4bab, since LLVM 18.1.3, specifying a custom LIBCXX_INSTALL_MODULES_DIR like this should work.) So @huangqinjin, if you have time to try it out with the bleeding edge CMake, the toolchains built in https://github.com/mstorsjo/llvm-mingw/actions/runs/8718827994 (once it completes) should include the necessary files.

huangqinjin · 2024-04-17T17:13:05Z

I tested the toolchain, there are two problems.

CMake cannot find clang-scan-deps, see https://gitlab.kitware.com/cmake/cmake/-/blob/v3.29.0/Modules/Compiler/Clang-FindBinUtils.cmake#L49.
CMake requires prefixed tool names e.g. x86_64-w64-mingw32-clang-scan-deps.
After manually creating a symlink, the toolchain can successfully build some example projects in repo https://github.com/huangqinjin/cxxmodules with CMake 3.28+.
libc++ std modules are incompatible with C headers shipped in the toolchain. Many errors like

n file included from /opt/llvm-mingw/share/libc++/v1/std.cppm:240:
/opt/llvm-mingw/share/libc++/v1/std/ctime.inc:20:14: error: using declaration referring to 'ctime' with internal linkage cannot be exported
   20 |   using std::ctime;
      |              ^
/opt/llvm-mingw/x86_64-w64-mingw32/include/time.h:267:33: note: target of using declaration
  267 | static __inline char *__CRTDECL ctime(const time_t *_Time) { return _ctime64(_Time);

The MinGW C headers need to be fixed to be exportable, i.e. use inline instead of static.

nolange · 2024-04-17T18:30:07Z

thanks for testing, for 1., you could try using a cmake toolchainfile: #412 (comment)

for 2., dropping static might give you some ABI issues. probably needs some c++ wrappers instead of using

mstorsjo · 2024-04-17T20:48:09Z

I tested the toolchain, there are two problems.

CMake cannot find clang-scan-deps, see https://gitlab.kitware.com/cmake/cmake/-/blob/v3.29.0/Modules/Compiler/Clang-FindBinUtils.cmake#L49.
CMake requires prefixed tool names e.g. x86_64-w64-mingw32-clang-scan-deps.
After manually creating a symlink, the toolchain can successfully build some example projects in repo https://github.com/huangqinjin/cxxmodules with CMake 3.28+.

Thanks, I see!

Does the target triple play any role in the invocation of clang-scan-deps, i.e. does this tool need to find the sysroot headers?

I'm wondering if plain symlinks are enough, or if we'd need to create a fullblown wrapper script/executable for this.

Some tools, like regular clang itself, can infer the target triple implicitly, when invoked via a symlink as <triple>-clang. If inferring the target triple is enough, and no other options are needed, this is enough. However on Windows, we don't use symlinks for these executables - so that would end up duplicating the clang-scan-deps.exe executable a couple times. As long as we link against libclang-cpp.dll and libLLVM-<version>.dll, the executable isn't huge (it seems to be around 175 KB in one build), but it's still a bit excessive. So for many such cases, I'm using https://github.com/mstorsjo/llvm-mingw/blob/master/wrappers/llvm-wrapper.c as a way to have a minimal-ish executable, 14 KB, that just executes the unprefixed executable - but that loses the implicit reading of the target prefix from a symlink. Then again, on Windows, the default target triple is a mingw triple anyway, so it probably would work just fine in any case.

OTOH, if the clang-scan-deps tool really is target agnostic, perhaps the CMake rule should be extended to look for an unprefixed clang-scan-deps executable as well?

libc++ std modules are incompatible with C headers shipped in the toolchain. Many errors like

n file included from /opt/llvm-mingw/share/libc++/v1/std.cppm:240:
/opt/llvm-mingw/share/libc++/v1/std/ctime.inc:20:14: error: using declaration referring to 'ctime' with internal linkage cannot be exported
   20 |   using std::ctime;
      |              ^
/opt/llvm-mingw/x86_64-w64-mingw32/include/time.h:267:33: note: target of using declaration
  267 | static __inline char *__CRTDECL ctime(const time_t *_Time) { return _ctime64(_Time);

The MinGW C headers need to be fixed to be exportable, i.e. use inline instead of static.

Oh, I see, thanks! This will indeed be a significant amount of work. There are a number of different patterns used for inline wrappers in the mingw-w64 headers, and they can be quite fiddly to get right, but I guess something like static inline for C and plain inline for C++ might be the best here, e.g. like the macro set up here: https://github.com/mingw-w64/mingw-w64/blob/master/mingw-w64-headers/crt/_mingw_mac.h#L285-L295

huangqinjin · 2024-04-18T06:24:36Z

After testing clang-scan-deps with different target triples, I believe that clang-scan-deps cannot automatically find out the target triple, and -target option is a must.

`test-deps.cpp`

#if defined(__x86_64__)
import x64;
#elif defined(__i386__)
import x86;
#elif defined(__aarch64__)
import arm64;
#elif defined(__arm__)
import arm;
#endif

without `-target`

archs=(x86_64 i686 aarch64 armv7)

for a in ${archs[@]}; do
  for b in ${archs[@]}; do
    echo "$a $b"
    /opt/llvm-mingw/bin/$a-w64-mingw32-clang-scan-deps -format=p1689 -- /opt/llvm-mingw/bin/$b-w64-mingw32-clang++ -std=c++23 -c test-deps.cpp
  done
done

All triple combinations output same, i.e. depends on x64 module (logical-name in requires[]), it is obviously wrong.

{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "x64"
        }
      ]
    }
  ],
  "version": 1
}

with `-target`

archs=(x86_64 i686 aarch64 armv7)

for a in ${archs[@]}; do
  for b in ${archs[@]}; do
    echo "$a $b"
    /opt/llvm-mingw/bin/$a-w64-mingw32-clang-scan-deps -format=p1689 -- /opt/llvm-mingw/bin/$b-w64-mingw32-clang++ --target=$b-w64-mingw32 -std=c++23 -c test-deps.cpp
  done
done

x86_64 x86_64
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "x64"
        }
      ]
    }
  ],
  "version": 1
}
x86_64 i686
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "x86"
        }
      ]
    }
  ],
  "version": 1
}
x86_64 aarch64
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "arm64"
        }
      ]
    }
  ],
  "version": 1
}
x86_64 armv7
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "arm"
        }
      ]
    }
  ],
  "version": 1
}
i686 x86_64
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "x64"
        }
      ]
    }
  ],
  "version": 1
}
i686 i686
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "x86"
        }
      ]
    }
  ],
  "version": 1
}
i686 aarch64
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "arm64"
        }
      ]
    }
  ],
  "version": 1
}
i686 armv7
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "arm"
        }
      ]
    }
  ],
  "version": 1
}
aarch64 x86_64
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "x64"
        }
      ]
    }
  ],
  "version": 1
}
aarch64 i686
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "x86"
        }
      ]
    }
  ],
  "version": 1
}
aarch64 aarch64
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "arm64"
        }
      ]
    }
  ],
  "version": 1
}
aarch64 armv7
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "arm"
        }
      ]
    }
  ],
  "version": 1
}
armv7 x86_64
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "x64"
        }
      ]
    }
  ],
  "version": 1
}
armv7 i686
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "x86"
        }
      ]
    }
  ],
  "version": 1
}
armv7 aarch64
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "arm64"
        }
      ]
    }
  ],
  "version": 1
}
armv7 armv7
{
  "revision": 0,
  "rules": [
    {
      "primary-output": "test-deps.o",
      "requires": [
        {
          "logical-name": "arm"
        }
      ]
    }
  ],
  "version": 1
}

huangqinjin · 2024-04-18T06:38:23Z

I think all options to clang must be explicitly specified in command lines, custom wrappers over plain clang do not work with clang-scan-deps.

mstorsjo · 2024-04-18T07:59:27Z

Thanks for looking into this, and thanks for providing examples for how to use and investigate this.

With this in mind, it does indeed look like we'll need to provide a wrapper script/executable for clang-scan-deps in order to get this right. (It might, practically, mostly work for most cases even without it, but let's get this right from the start.)

nolange force-pushed the install_libcxx_modules branch from cffd8b5 to e04f18a Compare March 14, 2024 10:06

nolange changed the title ~~Install libcxx modules~~ Install clang-scan-deps tool Mar 14, 2024

mstorsjo reviewed Apr 12, 2024

View reviewed changes

build-llvm.sh Outdated Show resolved Hide resolved

mstorsjo force-pushed the install_libcxx_modules branch from e04f18a to 16d7aca Compare April 12, 2024 08:06

mstorsjo merged commit 7f80719 into mstorsjo:master Apr 12, 2024
19 checks passed

nolange deleted the install_libcxx_modules branch April 12, 2024 21:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Install clang-scan-deps tool #412

Install clang-scan-deps tool #412

nolange commented Mar 13, 2024

mstorsjo commented Mar 13, 2024

mstorsjo commented Mar 13, 2024

nolange commented Mar 13, 2024

nolange commented Mar 13, 2024

mstorsjo commented Mar 13, 2024

mstorsjo commented Apr 17, 2024

huangqinjin commented Apr 17, 2024

nolange commented Apr 17, 2024

mstorsjo commented Apr 17, 2024

huangqinjin commented Apr 18, 2024

huangqinjin commented Apr 18, 2024

mstorsjo commented Apr 18, 2024

Install clang-scan-deps tool #412

Install clang-scan-deps tool #412

Conversation

nolange commented Mar 13, 2024

mstorsjo commented Mar 13, 2024

mstorsjo commented Mar 13, 2024

nolange commented Mar 13, 2024

nolange commented Mar 13, 2024

mstorsjo commented Mar 13, 2024

mstorsjo commented Apr 17, 2024

huangqinjin commented Apr 17, 2024

nolange commented Apr 17, 2024

mstorsjo commented Apr 17, 2024

huangqinjin commented Apr 18, 2024

test-deps.cpp

without -target

with -target

huangqinjin commented Apr 18, 2024

mstorsjo commented Apr 18, 2024

`test-deps.cpp`

without `-target`

with `-target`