xmake icon indicating copy to clipboard operation
xmake copied to clipboard

Improve C++20 modules support

Open waruqi opened this issue 3 years ago • 12 comments

https://github.com/xmake-io/xmake/pull/2623

waruqi avatar Aug 03 '22 09:08 waruqi

@Arthapz I have improved some codes. e.g. toolchain includedirs, fix bmi flags for msvc, format code and optimize performance...

I still need to understand the code in depth, maybe it will take some time.

waruqi avatar Aug 04 '22 03:08 waruqi

i'm continuously testing it as i migrated one of my projet to C++20 modules

Arthapz avatar Aug 04 '22 16:08 Arthapz

https://github.com/Arthapz/xmake/commit/32d0bbebf81e59f156769a43c5ae36181eadb040 we should backport this to avoid errors like

..\..\build\debug\stormkit\.gens\model_viewer\windows\x64\debug\engine\applications\ModelViewer\src\moc_StormKitNode.cpp: fatal error C1083: Cannot open compiler generated file:
     '..\..\build\debug\stormkit\.gens\model_viewer\windows\x64\debug\rules\modules\cache\..\..\..\..\..\build\debug\stormkit\.gens\model_viewer\windows\x64\debug\engine\applications\ModelViewer\src\moc_StormKitNode.cpp.json': No such file or directory

Arthapz avatar Aug 04 '22 17:08 Arthapz

https://github.com/Arthapz/xmake/commits/cxxmodules i'm pushing on this branch now

Arthapz avatar Aug 04 '22 21:08 Arthapz

https://github.com/Arthapz/xmake/commits/cxxmodules i'm pushing on this branch now

you can open a pr to cxxmodules branch

waruqi avatar Aug 04 '22 23:08 waruqi

This patch has some problems with the use of batchcmds at the moment.

About parallel compilation

All batchcmds:vrunv tasks added to before_buildcmd_files are executed serially, which prevents the modules from being compiled in parallel as much as possible.

Although serial compilation will always ensure that modules are compiled correctly, we need to increase the speed of compilation of modules as much as possible.

In the previous implementation, with module_parser.build_batchjobs in before_build_files, was able to compile in parallel as much as possible while keeping the dependencies intact.

However, the implementation of before_buildcmd_files is now necessary to provide the possibility of intelligense/compile_commands support later.

However, we can continue to add before_build_files to optimise parallel compilation.

  • xmake build -> before_build_files -> modules parallel compilation
  • xmake project -k compile_commands/cmake/vs -> before_buildcmd_files -> serial compilation of modules

batchcmds:set_depmtime and batchcmds:set_depcache

they can only be set once, it cannot be set more than once in a loop, as a batchcmds is a complete set of independent serial tasks.

They can only be executed or ignored at the same time

waruqi avatar Aug 05 '22 01:08 waruqi

However, we can continue to add before_build_files to optimise parallel compilation.

xmake build -> before_build_files -> modules parallel compilation xmake project -k compile_commands/cmake/vs -> before_buildcmd_files -> serial compilation of modules

and how we do the switch between before_build_files and before_buildcmd_files ?

Arthapz avatar Aug 05 '22 02:08 Arthapz

We don't need to switch it, if before_build_files doesn't exist then xmake build and project generator will always use before_buildcmd_files.

If we also define before_build_files, then xmake build will use before_build_files instead of before_build_files.

so, we can optimise build modules for xmake build by providing before_build_files, for example to provide better parallel compilation.

waruqi avatar Aug 05 '22 02:08 waruqi

Of course, we can optimize the implementation of before_build_files later, but for now we'll give priority to improving the implementation of before_buildcmds_files.

waruqi avatar Aug 05 '22 02:08 waruqi

I implemented a module mapper file for MSVC, it simplifies the code a little and i think i can do the same for clang

Arthapz avatar Aug 06 '22 04:08 Arthapz

mapper file does not work for clang. @Arthapz

> $ rm -rf .xmake/; rm -rf build/; xmake f --toolchain=llvm -c;xmake -j1
checking for platform ... macosx
checking for architecture ... x86_64
checking for Xcode directory ... /Applications/Xcode.app
checking for SDK version of Xcode for macosx (x86_64) ... 11.3
[ 20%]: generating.cxx.module.deps src/main.cpp
[ 20%]: generating.cxx.module.deps src/foo.mpp
[ 20%]: generating.cxx.module.deps src/bar.mpp
[ 20%]: generating.cxx.module.deps src/cat.mpp
[ 20%]: generating.cxx.module.deps src/zoo.mpp
[ 20%]: generating.cxx.module.bmi zoo
[ 20%]: generating.cxx.module.bmi cat
[ 20%]: generating.cxx.module.bmi bar
error: src/bar.mpp:2:8: fatal error: module 'zoo' not found
import zoo;
~~~~~~~^~~
1 error generated.
> error: src/hello_impl.cpp:4:1: error: definition of module 'hello' is not av
ailable; use -fmodule-file= to specify path to precompiled module interface
module hello;
^
src/hello_impl.cpp:5:8: fatal error: module 'mod' not found
import mod;
~~~~~~~^~~
2 errors generated

waruqi avatar Aug 08 '22 13:08 waruqi

$ xmake clean -a; xmake f --toolchain=clang -c; xmake -rv -j1          
checking for platform ... linux
checking for architecture ... x86_64
[  0%]: generating.cxx.module.deps src/hello_impl.cpp
[  0%]: generating.cxx.module.deps src/main.cpp
[  0%]: generating.cxx.module.deps src/mod_impl.cpp
[  0%]: generating.cxx.module.deps src/hello.mpp
[  0%]: generating.cxx.module.deps src/mod.mpp
checking for flags (clang_modules_cache_path) ... ok
checking for flags (clang_emit_module_interface) ... ok
checking for flags (clang_module_file) ... ok
[ 41%]: generating.cxx.module.bmi mod
checking for flags (-std=c++20) ... ok
checking for flags (-fmodules) ... ok
/usr/bin/clang -Qunused-arguments -m64 -std=c++20 -fmodules -fbuiltin-module-map -fimplicit-modules -fno-implicit-module-maps -fmodule-file=mod=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/mod.pcm -fmodule-file=hello=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/hello.pcm -fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache -emit-module-interface -c -x c++-module --precompile src/mod.mpp -o build/.gens/dependence/linux/x86_64/release/rules/modules/cache/mod.pcm
/usr/bin/clang -Qunused-arguments -m64 -std=c++20 -fmodules -fbuiltin-module-map -fimplicit-modules -fno-implicit-module-maps -fmodule-file=mod=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/mod.pcm -fmodule-file=hello=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/hello.pcm -fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache build/.gens/dependence/linux/x86_64/release/rules/modules/cache/mod.pcm -c -o build/.objs/dependence/linux/x86_64/release/src/mod.mpp.o
[ 50%]: generating.cxx.module.bmi hello
/usr/bin/clang -Qunused-arguments -m64 -std=c++20 -fmodules -fbuiltin-module-map -fimplicit-modules -fno-implicit-module-maps -fmodule-file=mod=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/mod.pcm -fmodule-file=hello=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/hello.pcm -fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache -emit-module-interface -c -x c++-module --precompile src/hello.mpp -o build/.gens/dependence/linux/x86_64/release/rules/modules/cache/hello.pcm
/usr/bin/clang -Qunused-arguments -m64 -std=c++20 -fmodules -fbuiltin-module-map -fimplicit-modules -fno-implicit-module-maps -fmodule-file=mod=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/mod.pcm -fmodule-file=hello=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/hello.pcm -fmodules-cache-path=build/.gens/dependence/linux/x86_64/release/rules/modules/cache build/.gens/dependence/linux/x86_64/release/rules/modules/cache/hello.pcm -c -o build/.objs/dependence/linux/x86_64/release/src/hello.mpp.o
[ 58%]: ccache compiling.release src/hello_impl.cpp
/usr/bin/clang -c -Qunused-arguments -m64 -std=c++20 -fmodules -fbuiltin-module-map -fimplicit-modules -fno-implicit-module-maps -fmodule-file=mod=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/mod.pcm -fmodule-file=hello=build/.gens/dependence/linux/x86_64/release/rules/modules/cache/hello.pcm -o build/.objs/dependence/linux/x86_64/release/src/hello_impl.cpp.o src/hello_impl.cpp
checking for flags (-MMD -MF) ... ok
checking for flags (-fdiagnostics-color=always) ... ok
error: src/hello_impl.cpp:4:1: error: definition of module 'hello' is not available; use -fmodule-file= to specify path to precompiled module interface
module hello;
^
src/hello_impl.cpp:16:5: error: use of undeclared identifier 'say'
    say::say(int data) : data_{data} {
    ^
src/hello_impl.cpp:19:10: error: use of undeclared identifier 'say'
    void say::hello() {
         ^
3 errors generated.

src/hello_impl.cpp is implementation of hello module, if I change -fmodule-file=hello=xxx/hello.pcm to -fmodule-file=xxx/hello.pcm, it will work for clang.

waruqi avatar Aug 09 '22 08:08 waruqi

Excellent! BTW, https://github.com/alibaba/async_simple/tree/CXX20Modules is used C++20 Modules completely and it is OK to be compiled by clang15 now. I guess it might be a good test case for you.

ChuanqiXu9 avatar Aug 11 '22 08:08 ChuanqiXu9

Excellent! BTW, https://github.com/alibaba/async_simple/tree/CXX20Modules is used C++20 Modules completely and it is OK to be compiled by clang15 now. I guess it might be a good test case for you.

Thanks, there are still some improvements to be made now, and you can test this patch when it is merged.

waruqi avatar Aug 12 '22 14:08 waruqi

mmmmh, when building a shared lib with header units, we may have link error because the library consummer don't have access to the header units .obj, i think i need some more reading about header units to handle that we may handle them wrong

Arthapz avatar Aug 13 '22 15:08 Arthapz

mmmmh, when building a shared lib with header units, we may have link error because the library consummer don't have access to the header units .obj, i think i need some more reading about header units to handle that we may handle them wrong

well, you can add a shared test example in tests directory first.

waruqi avatar Aug 15 '22 03:08 waruqi

cmake generator work fine for gcc/clang now.

$ xmake project -k cmake; cd build/; cmake ..; make 
[  0%]: generating.cxx.module.deps src/hello_impl.cpp
[  0%]: generating.cxx.module.deps src/main.cpp
[  0%]: generating.cxx.module.deps src/mod_impl.cpp
[  0%]: generating.cxx.module.deps src/hello.mpp
[  0%]: generating.cxx.module.deps src/mod.mpp
create ok!
-- The CXX compiler identification is GNU 11.3.0
-- The C compiler identification is GNU 11.3.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Configuring done
-- Generating done
-- Build files have been written to: /mnt/xmake/tests/projects/c++/modules/dependence/build
[ 20%] Generating output_dependence_ADE56AE0
[ 0%]: generating.cxx.module.bmi mod
[ 0%]: generating.cxx.module.bmi hello
[ 20%] Built target target_dependence_ADE56AE0
[ 40%] Building CXX object CMakeFiles/dependence.dir/src/hello_impl.cpp.o
[ 60%] Building CXX object CMakeFiles/dependence.dir/src/main.cpp.o
[ 80%] Building CXX object CMakeFiles/dependence.dir/src/mod_impl.cpp.o
[100%] Linking CXX executable linux/x86_64/release/dependence
[100%] Built target dependence

waruqi avatar Aug 15 '22 15:08 waruqi

The vs generator is not currently supported because it requires all cxxflags to be fetched in advance of the config phase, but some of the module's cxxflags are dynamically added in before_build_files, so they cannot be fetched.

This patch is a bit large and works mostly fine for now, so I've merged it for now, other improvements to vs generator and shared libs can be improved in a new patch separately.

waruqi avatar Aug 16 '22 04:08 waruqi