STEllAR-GROUP / hpx

The C++ Standard Library for Parallelism and Concurrency
https://hpx.stellar-group.org
Boost Software License 1.0
2.51k stars 427 forks source link

Support C++20 modules #6014

Open hkaiser opened 1 year ago

hkaiser commented 1 year ago

Level 0:

Level 1:

Level 2:

Level 3:

Level 4:

Level 5:

Level 6:

Level 7:

Level 8:

Level 9:

Level 10:

Level 11:

Level 12:

Level 13:

Level 14:

Level 15:

Level 16:

Level 17:

Level 18:

Level 19:

Level 20:

Level 21:

Level 22:

Level 23:

Level 24:

Level 25:

Level 26:

Level 27:

Level 28:

Level 29:

Level 30:

Level 31:

Level 32:

Level 33:

Level 34:

Level 35:

Level 36:

Level 37:

Level 38:

Johan511 commented 1 year ago

Hi, I am interested on working on this enhancement. Can you suggest any resources I can read upon to better help with this issue?

hkaiser commented 1 year ago

I am interested on working on this enhancement.

@Johan511 Perfect!

Can you suggest any resources

I'd suggest watching Daniela Egert's talks first, e.g.:

Also, I have started working on this a while back (see: https://github.com/hkaiser/hpx/tree/c++20_modules). But had given up because of problems with the cmake support for modules. I think they have made some progress implementing things, though (https://www.youtube.com/watch?v=5X803cXe02Y).

Johan511 commented 1 year ago

Thank you @hkaiser . I have gone through the talks you have suggested and also through the commit you have made to the c++_20_modules branch.

Apart from the changes made to the cmake files most of the other changes seem to be replacing includes from #include <hpx/abc/abc.h> to #include <hpx/module/____.h> which as of now has the same effect. But later on, I believe the hpx/module/___.h file will later import a module if the corresponding HPX_HAVE_MODULE is defined, else would include the header files <hpx/abc/abc.h> and cmake will take care of linking the right object files.

I hope I have properly understood what is being done. What do you think would be good sub task for me to start working on?

hkaiser commented 1 year ago

Apart from the changes made to the cmake files most of the other changes seem to be replacing includes from #include <hpx/abc/abc.h> to #include <hpx/module/____.h> which as of now has the same effect. But later on, I believe the hpx/module/___.h file will later import a module if the corresponding HPX_HAVE_MODULE is defined, else would include the header files <hpx/abc/abc.h> and cmake will take care of linking the right object files.

Tes, that's roughtly what happens. Note, the <hpx/module/...> files are being generated by cmake.

I hope I have properly understood what is being done. What do you think would be good sub task for me to start working on?

I'd start with making the build system do the right thing (mostly in cmake/HPX_AddModule.cmake). My assumption was that each of the HPX modules (as listed above) should expose their own C++ module (possibly a module part). Those module parts should be composed into two C++ modules that will be consumed by users: one corresponding to the current hpx_core, the other corresponding to the full hpx binary libraries.

Thus, the way forward could be to adapt each of the HPX modules one by one from the bottom up. I don't think that that all of HPX need to be adapted.

The other thing I would like to achieve is that HPX should be usable without C++ modules for older compilers or by users that don't want to work with C++ modules (yet).

Johan511 commented 1 year ago

sure, I will try going through HPX_AddModule.cmake

The other thing I would like to achieve is that HPX should be usable without C++ modules for older compilers or by users that don't want to work with C++ modules (yet).

How are we going to achieve this? won't older compilers throw an error on encountering the export keyword? I was thinking about using #ifdef, #ifndef directives to achieve it but am not sure if its the ideal way.

hkaiser commented 1 year ago

How are we going to achieve this? won't older compilers throw an error on encountering the export keyword? I was thinking about using #ifdef, #ifndef directives to achieve it but am not sure if its the ideal way.

The generated module header files will have a #if defined(HPX_HAVE_MODULES) that either #includes all module headers (as before) or import the module.

Johan511 commented 1 year ago

I have gone through HPX_AddModule.cmake, the add_hpx_module function seems to be the one which generates the files included by <hpx/module/...> or at least sets necessary contents of the files in various variables, I haven't been able to find the file ( WRITE ) command which writes to the .hpp file

Those module parts should be composed into two C++ modules that will be consumed by users: one corresponding to the current hpx_core, the other corresponding to the full hpx binary libraries.

I believe this the following code deals with importing the core and full binary libraries ` set(cppm_module_postfix "${cppm_module_postfix}#else\n\n")

  set(cppm_module_postfix
      "${cppm_module_postfix}#if defined(HPX_CORE_EXPORTS)\n"
  )

  set(cppm_module_postfix
      "${cppm_module_postfix}import hpx.${libname}.${modulename};\n"
  )

  set(cppm_module_postfix "${cppm_module_postfix}#else\n")

  set(cppm_module_postfix "${cppm_module_postfix}import hpx.core;\n")

  set(cppm_module_postfix
      "${cppm_module_postfix}#endif // HPX_CORE_EXPORTS\n\n"
  )

  set(cppm_module_postfix
      "${cppm_module_postfix}#endif // HPX_HAVE_CXX20_MODULES\n"
  )`

Can you please guide me on what hpx_core is, and how it is different from the hpx binary libraries.

add_hpx_module_library( ${libname} ${modulename} LINKTYPE ${module_library_type} MODULE_SOURCES ${module_sources} MODULE_HEADERS ${headers} SOURCES ${sources} ${config_entries_source} OBJECTS ${${modulename}_OBJECTS} HEADERS ${generated_headers} ${compat_headers} FOLDER "Core/Modules/${libname_cap}/BMI")

This function seems to be the one which should generate the required .bmi files, as of now most it its definition is commented out. Do we need to work on it to generate the required .bmi files.

if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") As of now we seem to allow modules only for MSVC, clang seems to introducing module support too. Do we have any other variable to verify if the compiler supports modules?

hkaiser commented 1 year ago

I have gone through HPX_AddModule.cmake, the add_hpx_module function seems to be the one which generates the files included by <hpx/module/...> or at least sets necessary contents of the files in various variables, I haven't been able to find the file ( WRITE ) command which writes to the .hpp file

It's written here: https://github.com/STEllAR-GROUP/hpx/blob/aa8143a28f9c29d27861250e53ef58f0917c6973/cmake/HPX_AddModule.cmake#L156-L159

Can you please guide me on what hpx_core is, and how it is different from the hpx binary libraries.

hpx_core.so/hpx_core.dll is one of the HPX binaries generated, essentially from all HPX sub-modules in libs/core. The other binary (hpx.so/hpx.dll) is generated from all HPX sub-modules in libs/full.

This function seems to be the one which should generate the required .bmi files, as of now most it its definition is commented out. Do we need to work on it to generate the required .bmi files.

Yes.

if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") As of now we seem to allow modules only for MSVC, clang seems to introducing module support too. Do we have any other variable to verify if the compiler supports modules?

HPX_WITH_CXX20_MODULES is the top level (user controlled) cmake variable. And yes, so far I have played with MSVC only, other compilers will most likely need different settings... The idea was to hide all the differences here: https://github.com/hkaiser/hpx/blob/c%2B%2B20_modules/cmake/HPX_AddModuleLibrary.cmake

Johan511 commented 1 year ago

I have made some changes to the add_hpx_module_library cmake function, can someone please look into it. Also can we have a separate branch to merge all our changes related to supporting modules into.

hkaiser commented 1 year ago

I have made some changes to the add_hpx_module_library cmake function, can someone please look into it.

I will, thanks!

Also can we have a separate branch to merge all our changes related to supporting modules into.

What do you mean by that? You already have created a PR from that branch, haven't you?

Johan511 commented 1 year ago

I am currently trying to merge into STEllAR-GROUP:hpx:master, I was suggesting that we could add module support in a separate branch say STEllAR-GROUP:hpx:modules and finally merge that into master. Also, I am currently working with a clone of the fork you had created to implement module support, I hope that is fine.

hkaiser commented 1 year ago

@Johan511 Sure, that works. I almost think it might be easier to start over from master and step by step reapply necessary changes, i.e. add module support to the build system, and then starting from the lowest level HPX modules convert those to use C++ modules; one by one.

Johan511 commented 1 year ago

Sure, that works. I almost think it might be easier to start over from master and step by step reapply necessary changes

I also feel the same, it might be better to branch from the current master and go through with the same/similar changes. There seem to be a lot of merge conflicts to deal with.

Johan511 commented 1 year ago

@hkaiser do you suggest I fork master and start working on it? where do you think I should start?

hkaiser commented 1 year ago

@hkaiser do you suggest I fork master and start working on it? where do you think I should start?

Yes, I'd start with creating a new branch and reapplying the build system changes. Then I'd work on exposing the first two or three HPX modules as C++ modules (core/preprocessor, core/config_registry, core/config) with the goal to make all tests pass. These steps will help to figure out what needs to be done, overall.

Johan511 commented 1 year ago

Sorry for the delay, I have gone through the master branch and tried to figure out to what extent modules have been implemented. There seems to be a cmake file HPX_AddModule.cmake which seems to be aimed at generating the modules. Can you confirm if it works as intended?

I still am unsure how we are exporting the components of the module we intend to export. My understanding has been that to export at function foo, we use export int foo (); How do we add this "export" keyword into the required files?

Can you suggest an environment to test out if my commits are working as intended? I mostly work with Linux (Ubuntu), do I need to shift to Visual Studio on windows to work on this, I have tried using a VM but the performance was not ideal.

hkaiser commented 1 year ago

Sorry for the delay, I have gone through the master branch and tried to figure out to what extent modules have been implemented. There seems to be a cmake file HPX_AddModule.cmake which seems to be aimed at generating the modules. Can you confirm if it works as intended?

There is a clash in terminology that needs to be clarified, I believe. On one hand, we have the HPX modules (those are not C++20 modules - yet). The HPX modules are a result of code refactorings performed over the last time to build a non-cyclic hierarchy of code blocks that allow to reason about structure in a better way. Each of the sub-directories in libs/core and libs/full comprises an HPX module. We generate a dependency report with each commit (e.g. https://hpx-docs.stellar-group.org/branches/master/report/index.html) that allows to navigate that hierarchy.

The HPX modules are linked into two binary libraries, all from core produce libhpx_core.so, and all HPX modules from full produce libhpx.so.

On the other hand, there are C++20 modules. The initial idea was to convert each of the HPX modules into its own C++20 module. If this is done step by step, from the bottom up, we should be able to modularize (C++20 modules) HPX in smaller pieces and still be able to build the whole thing (i.e. tests should pass after each change).

The second part of this idea was that we would like to leave using the C++20 modules optional, so that people could still use HPX without opting into using C++ modules. This should also keep HPX compatible with C++17.

I still am unsure how we are exporting the components of the module we intend to export. My understanding has been that to export at function foo, we use export int foo (); How do we add this "export" keyword into the required files?

I started experimenting with C++20 modules in my PR. The initial thought was to utilize header units. As each of the HPX modules already has a (generated) module header (e.g. #include <hpx/modules/preprocessor.hpp>), where currently each of those modules simply includes all of the related HPX headers that belong to that module. For instance:

#pragma once

#include <hpx/preprocessor/cat.hpp>
#include <hpx/preprocessor/config.hpp>
#include <hpx/preprocessor/expand.hpp>
#include <hpx/preprocessor/identity.hpp>
#include <hpx/preprocessor/nargs.hpp>
#include <hpx/preprocessor/stringize.hpp>
#include <hpx/preprocessor/strip_parens.hpp>

You can find those generated module headers under <build_dir>/libs/core/<module_name>/include/hpx/modules.

In my PR, I changed this to look like:

#pragma once

#if !defined(HPX_HAVE_CXX20_MODULES)

#include <hpx/preprocessor/cat.hpp>
#include <hpx/preprocessor/expand.hpp>
#include <hpx/preprocessor/identity.hpp>
#include <hpx/preprocessor/nargs.hpp>
#include <hpx/preprocessor/stringize.hpp>
#include <hpx/preprocessor/strip_parens.hpp>

#else
import hpx.core.preprocessor;
#endif

IOW, if used with modules, #including <hpx/modules/preprocessor.hpp> will import the corresponding C++ module, otherwise it will stick to the old way of #including the original header files.

These headers should eventually be generated, but in the PR those were added explicitly (manually) (e.g. https://github.com/hkaiser/hpx/blob/c%2B%2B20_modules/libs/core/preprocessor/include/hpx/modules/preprocessor.hpp).

This scheme also implies that throughout the whole HPX code base, no reference to any of the separate #include's should be made, but all #include's should refer to the generated module header only (except for headers from inside the same HPX module).

Also, each HPX module now has added a special .cppm module definition file, that instructs the compiler to generate the C++20 module exports, etc. For instance (https://github.com/hkaiser/hpx/blob/c%2B%2B20_modules/libs/core/preprocessor/src/hpx.core.preprocessor.cppm):

export module hpx.core.preprocessor;

export import <hpx/preprocessor/cat.hpp>;
export import <hpx/preprocessor/expand.hpp>;
export import <hpx/preprocessor/identity.hpp>;
export import <hpx/preprocessor/nargs.hpp>;
export import <hpx/preprocessor/stringize.hpp>;
export import <hpx/preprocessor/strip_parens.hpp>;

These files could eventually be generated by cmake as well, but for now I have added those manually.

Can you suggest an environment to test out if my commits are working as intended? I mostly work with Linux (Ubuntu), do I need to shift to Visual Studio on windows to work on this, I have tried using a VM but the performance was not ideal.

I have no experience with C++ modules when using gcc or clang. My experiments were using MSVC and Visual Studio. I'm not able to suggest anything here.

gonidelis commented 1 year ago

https://www.kitware.com/import-cmake-c20-modules

Johan511 commented 1 year ago

I have been experimenting with modules with GNU and LLVM compilers too and there do seem to be a few differences related to how they generate module files. As per my understanding GNU seems to generate only .o files which also act as pre-compiled module files, whereas LLVM seems to not expose its modules in its .o files, we need to separately generate .pcm files

In case of GNU I was planning to compile HPX modules into a library using something similar to "add_library(mylib STATIC math.cpp)". I have tried doing it for some smaller examples and linked the libraries together with the final executable, and it has worked as expected. I am trying to do the same for clang.

I haven't been able to go through the current build process for HPX nor have worked with large scale projects in the past so I hope to receive feedback on if this would be a suggested way of doing it.

A statement I would like to confirm is that, does HPX finally compile all it's code into 2 binary libraries (namely libhpx_core.so and libhpx.so) and the end user is supposed to link these libraries into their project? Is there any reason against providing more disjoint libraries so the user can link them as per their use case? I believe it might decrease the final size of the executable for the user. Do correct me if this is unfeasible or straight up incorrect.

Johan511 commented 1 year ago

Hi @gonidelis, thank you for the resource. One thing I would like to point out is that they suggest using the FILE_SET option which seem to be introduced only in CMAKE 3.25, are we comfortable using recently pushed features?

hkaiser commented 1 year ago

Hi @gonidelis, thank you for the resource. One thing I would like to point out is that they suggest using the FILE_SET option which seem to be introduced only in CMAKE 3.25, are we comfortable using recently pushed features?

Yes. People interested in using C++ modules will have to install recent versions of cmake. By the time the dust has settled this will not be a problem anymore (if it ever was).

hkaiser commented 1 year ago

BTW, in addition to my comment above.

The HPX Modules core/preprocessor and core/config_registry are special, as those are the only ones that do not depend on the HPX module config/config (see https://hpx-docs.stellar-group.org/branches/master/report/master/module-levels.html). For this reason, the module headers for those two are handcrafted (not generated by cmake).

For all HPX modules that depend on config, the module files should be generated by cmake. For instance:

#pragma once

#include <hpx/config.hpp>

#if !defined(HPX_HAVE_CXX20_MODULES)

#if defined(HPX_HAVE_MODULE_CACHE)
#include <hpx/cache/entries/entry.hpp>
#include <hpx/cache/entries/fifo_entry.hpp>
#include <hpx/cache/entries/lfu_entry.hpp>
#include <hpx/cache/entries/lru_entry.hpp>
#include <hpx/cache/entries/size_entry.hpp>
#include <hpx/cache/local_cache.hpp>
#include <hpx/cache/lru_cache.hpp>
#include <hpx/cache/policies/always.hpp>
#include <hpx/cache/statistics/local_full_statistics.hpp>
#include <hpx/cache/statistics/local_statistics.hpp>
#include <hpx/cache/statistics/no_statistics.hpp>
#endif // HPX_HAVE_MODULE_CACHE

#elif defined(HPX_CORE_EXPORTS)
import hpx.core.cache;
#else
import hpx.core;
#endif

That means, that if C++ modules are not enabled, any file #including <hpx/modules/cache.hpp> will simply see all related headers for that module. If this happens when C++ modules are enabled while compiling the HPX core binary (HPX_HAVE_CXX20_MODULES and HPX_CORE_EXPORTS are defined), then this will import that cache module. All other code will import the whole HPX core binary module.

Johan511 commented 1 year ago

https://www.kitware.com/import-cmake-c20-modules

I have tried going through this, I have faced some errors. Mostly along the lines of CMake Error in CMakeLists.txt: The "export_bmi_and_interfaces" target has C++ module sources but its experimental support has not been requested

I am using cmake 3.25 so don't believe that was the issue.

I tried cmake on the examples in cmake repository and faced similar issues.

This happens during the cmake phase, so I don't believe it depends on the compiler.

Any idea about it?

Another error I did come across was FILE_SET only supporting HEADER as an option, I am unsure if I patched it accidentally or its just shadowed by the current error.

hkaiser commented 1 year ago

@Johan511 you might want to watch this video: https://www.youtube.com/watch?v=5X803cXe02Y. There the speaker explains how to enable experimental support for C++ modules (amongst other things). Essentially this:

image

Johan511 commented 1 year ago

I have gone through quite some material and reached out to people working on module support for C++ 20. Just wanted to document a couple of things over here, the following is the example I have tried experimenting with.

This is also a good resource the understand CMake support for C++ modules

1. GCC does not support scanning dependencies in its main version.

g++: error: unrecognized command-line option ‘-fdep-file=CMakeFiles/std_module_example.dir/t2.cxx.o.ddi’ g++: error: unrecognized command-line option ‘-fdep-output=CMakeFiles/std_module_example.dir/t2.cxx.o’ g++: error: unrecognized command-line option ‘-fdep-format=trtbd’

These are the errors the user would face, there is a patch which has implemented these command-line options but it hasn't worked for me. gcc-help believes there might be a mistake and has advised me to try again in the patch. I will try it on a container and update the same here.

2. GNU-make does not support modules sources

CMake Error in CMakeLists.txt: The "std_module_example" target contains C++ module sources which are not supported by the generator

Only Ninja (>1.12) supports it.

3. set(CMAKE_EXPERIMENTAL_CXX_MODULE_CMAKE_API "2182bf5c-ef0d-489a-91da-49dbc3090d2a") is the API which worked for me.

set(CMAKE_EXPERIMENTAL_CXX_MODULE_CMAKE_API "3c375311-a3c9-4396-a187-3227ef642046") has not worked for me.

CMake Error at CMakeLists.txt:20 (target_sources): target_sources File set TYPE may only be "HEADERS"

Johan511 commented 1 year ago

I have managed to get everything setup, and updated/added some cmake files to add c++20 module. To initially test it out, I am trying to generate a module of all the .hpp files in hpx/preprocessor I believe I need to link the module generated to libhpx_core.so , which cmake files should I look into to do it?

Also in spite of using set(HPX_WITH_CXX_STANDARD 20), cmake logs the following

-- Using C++17

How do I fix it?

hkaiser commented 1 year ago

I have managed to get everything setup, and updated/added some cmake files to add c++20 module. To initially test it out, I am trying to generate a module of all the .hpp files in hpx/preprocessor I believe I need to link the module generated to libhpx_core.so , which cmake files should I look into to do it?

the cmake library hpx_core is created here: https://github.com/STEllAR-GROUP/hpx/blob/e877f5d77ac8939a93084af6f70251c172247b3c/libs/CMakeLists.txt#L314

also, look for all mentions of hpx_core in the CMakeLists.txt files.

Also in spite of using set(HPX_WITH_CXX_STANDARD 20), cmake logs the following

-- Using C++17

How do I fix it?

Did you delete your cmake cache?

Johan511 commented 1 year ago

ninja_compile_logs.txt ninja_compile_success_logs.txt ninja_test_compile_logs.txt cmake_logs.txt

There seem to be issues, with simply trying to compile all the tests using the patches meant for module support. HPX compiles with lot of warnings, but tests for HPX don't even compile. I have attached the logs. Unfortunately diff does not work as ninja runs in parallel.

Most of the issues seem to be due to __atomic_load and _GLIBCXX_OPERATOR_DELETE.

grep for "/home/hhn/makes/gcc-modules-install" may help as that is where I have installed the GCC patches.

building tests fails with ninja: build stopped: subcommand failed.

Johan511 commented 1 year ago

The previous issue was with compiling tests, it was fixed with #include , still facing a few dependency scanning issues. I have sent a PR (it doesn't compile as modules don't seem to be getting built/found) to show the changes made. Any idea what might be the issue? As of now I am just hacking around trying to build a single module and get it to compile. Will work towards a cleaner build once I get I good understanding of what to do.

Johan511 commented 1 year ago

The previous issue was with compiling tests, it was fixed with #include , still facing a few dependency scanning issues. I have sent a PR (it doesn't compile as modules don't seem to be getting built/found) to show the changes made. Any idea what might be the issue? As of now I am just hacking around trying to build a single module and get it to compile. Will work towards a cleaner build once I get I good understanding of what to do.

@hkaiser any idea what the error might be?

jainl28patel commented 1 year ago

Hi @hkaiser , I am new here and want to contribute to hpx. I think this issue will be good to start with, if I am correct then can you please provide me with some resources else can you please guide me from where can I start to contribute to hpx. Thanks.

Johan511 commented 1 year ago

Hi @jainl28patel , I don't think this is the best first issue. As of now we aren't completely sure how to proceed with it. This might be a good set of resources to learn how to contribute.

hkaiser commented 1 year ago

@jainl28patel I'd agree with @Johan511. The issue here is that creating C++ modules requires to have a good understanding of the API to expose. I think we're not quite at a position where we can fully define this. Even if, at least partially, this could be done today (as far as the compatibility with the Standard is concerned).

The bigger obstacle for this is the non-uniform support for modules by the different compilers. MSVC support is decent, and I think we have a good understanding of the technical approach to minimize necessary code changes and still being able to use HPX without modules. I have a proof-of-concept implementation of exposing C++ modules for a very small subset of HPX for MSVC, but I'm not sure whether this would work for gcc and/or clang.

To summarize, I'd like to suggest leaving this topic alone for a while, at least until gcc and clang have caught up with their support of C++ modules.

What I could suggest you looked into however would be to make our implementations of small_vector, flat_set, and flat_map fully conforming to the respective standardization proposals (see std::inplace_vector and std::flat_map/std::flat_set).

Would that be something interesting to you?

jainl28patel commented 1 year ago

Thanks @Johan511

What I could suggest you looked into however would be to make our implementations of small_vector, flat_set, and flat_map fully conforming to the respective standardization proposals (see std::inplace_vector and std::flat_map/std::flat_set. Would that be something interesting to you?

@hkaiser Yes! I would like to work on this. Could you provide me with some resource or reference for the task. I am a bit new to developing C++ libraries, but I would love to learn this and implement. Thanks.

Johan511 commented 1 year ago

Could you provide me with some resource or reference for the task.

hkaiser has linked the accepted specification how these data structures should be.

If you are feeling lost in the code I would suggest the resources linked earlier.

Becoming more comfortable with C++ really helped me navigate HPX easily, hkaiser's lectures (point 5 in resources page) can be really help out with it.

kelteseth commented 4 months ago

We now track hpx modules support progress at https://arewemodulesyet.org/ . Feel free to create a PR if the status changes at https://github.com/kelteseth/arewemodulesyet/ 😊