Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the default leap C++ compilation flag to -std=c++20 #1079

Closed
2 of 5 tasks
greg7mdp opened this issue Apr 24, 2023 · 3 comments · Fixed by #1458
Closed
2 of 5 tasks

Change the default leap C++ compilation flag to -std=c++20 #1079

greg7mdp opened this issue Apr 24, 2023 · 3 comments · Fixed by #1458

Comments

@greg7mdp
Copy link
Contributor

greg7mdp commented Apr 24, 2023

Prerequisites:

llvm issue background

This is a known build issue with the LLVM headers (gcc-12, -std=c++20), see this.

Manually changing /usr/include/llvm-11/llvm/ADT/STLExtras.h to remove the unneeded <R> allows it to build fine.

Issue is fixed in LLVM-13

Possible way forward

Investigate what part of leap depends on those llvm headers, and possibly keep building these with -std=c++17.

@greg7mdp greg7mdp changed the title Update the default leap C++ compilation flag to -std=c++20 Change the default leap C++ compilation flag to -std=c++20 Apr 24, 2023
@arhag
Copy link
Member

arhag commented Apr 25, 2023

We also want to run a replay (even before chicken test is ready) soon after making the C++20 switch.

@greg7mdp
Copy link
Contributor Author

greg7mdp commented Apr 26, 2023

Issue with g++-12 / std=c++20

Investigate what part of leap depends on those llvm headers, and possibly keep building these with -std=c++17.

I tried this, setting set_property(TARGET eosio_chain PROPERTY CXX_STANDARD 17) in libraries/chain/CMakeLists.txt.
While this does fix the compilation issue for the cpp files, it creates new link issues such as:

usr/bin/ld: libraries/chain/libeosio_chain.a(crypto.cpp.o): in function `eosio::chain::webassembly::interface::alt_bn128_mul(eosio::vm::span<char const, 18446744073709551615ul>, eosio::vm::span<char const, 18446744073709551615ul>, eosio::vm::span<char, 18446744073709551615ul>) const':
/home/greg/github/enf/leap/libraries/chain/webassembly/crypto.cpp:130: undefined reference to `bn256::g1_scalar_mul(nonstd::span_lite::span<unsigned char const, 64ul>, nonstd::span_lite::span<unsigned char const, 32ul>, nonstd::span_lite::span<unsigned char, 64ul>)'
/usr/bin/ld: libraries/chain/libeosio_chain.a(crypto.cpp.o): in function `eosio::chain::webassembly::interface::alt_bn128_pair(eosio::vm::span<char const, 18446744073709551615ul>) const':
/home/greg/github/enf/leap/libraries/chain/webassembly/crypto.cpp:139: undefined reference to `bn256::pairing_check(nonstd::span_lite::span<unsigned char const, 18446744073709551615ul>, std::function<void ()>)'
collect2: error: ld returned 1 exit status

I tried building with llvm-13 bit the headers have changed:

/home/greg/github/enf/leap/libraries/chain/webassembly/runtimes/eos-vm-oc/LLVMJIT.cpp:23:10: fatal error: llvm/ExecutionEngine/Orc/LambdaResolver.h: No such file or directory
   23 | #include "llvm/ExecutionEngine/Orc/LambdaResolver.h"

Also, even when patching the headers by hand, some tests fail in release mode with the g++-12 / std=c++20

The following tests FAILED:
	 94 - partitioned_block_log_unit_test_eos-vm-oc (SEGFAULT)
	 95 - partitioned_block_log_unit_test_eos-vm (SEGFAULT)
	 96 - partitioned_block_log_unit_test_eos-vm-jit (SEGFAULT)

However g++-10 / std=c++20 is fine - as is clang-16.

  • leap builds fine (and without a single warning). It is a thing of beauty.
  • tests all pass (release / debug)

Proposal

  1. standardize on g++-10 / clang-16 for compiler versions
  2. flip the switch to --std=c++20

@greg7mdp greg7mdp self-assigned this Apr 27, 2023
@bhazzard
Copy link

Blocked awaiting completion of Prerequisites (in description)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants