Skip to content

Commit

Permalink
tenstorrent#4252: Update to C++20
Browse files Browse the repository at this point in the history
- tenstorrent#4252: fixing c++20 compile errors
- tenstorrent#4252: remove stacktrace
- tenstorrent#4252: extra C++20 warning ignore flag
- tenstorrent#4252: don't push printing to runtime
- tenstorrent#4252: force Clang-17 as compiler since GCC-9 can't support all of C++20
    - also change install instructions to require Clang-17
- tenstorrent#4252: fix gcc11 build errors
  • Loading branch information
vtangTT committed Jun 5, 2024
1 parent 933e532 commit 43ad09f
Show file tree
Hide file tree
Showing 16 changed files with 58 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/build-artifact.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
git submodule update --init --recursive
- name: Build tt-metal and libs
run: |
cmake -B build -G Ninja -DCMAKE_CXX_COMPILER=clang++-17
cmake -B build -G Ninja
cmake --build build --target tests
cmake --build build --target install
- name: 'Tar files'
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
echo "TT_METAL_HOME=$(pwd)" >> $GITHUB_ENV
- name: Build tt-metal libraries
run: |
cmake -B build -G Ninja -DCMAKE_CXX_COMPILER=clang++-17
cmake -B build -G Ninja
cmake --build build
- name: Build tt-metal C++ tests
run: |
Expand Down
20 changes: 11 additions & 9 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ cmake_policy(VERSION 3.16)
# Project setup
############################################

### Uncomment this if you don't want to manually pass Clang-17 compiler through CLI ###
# find_program(CLANG_17 clang++-17)
# if(CLANG_17)
# message(STATUS "Found Clang-17 here: ${CLANG_17}")
# set(CMAKE_CXX_COMPILER "${CLANG_17}")
# else()
# message(WARNING "Clang++-17 not found, recommended to build with Clang-17 > GCC for better perf")
# endif()
# Use Clang-17 by default until we upgrade to Ubuntu version that supports higher GCC
# No longer support GCC-9 as it does not support C++20
find_program(CLANG_17 clang++-17)
if(CLANG_17)
message(STATUS "Found Clang-17 here: ${CLANG_17}")
set(CMAKE_CXX_COMPILER "${CLANG_17}")
else()
message(WARNING "Clang++-17 not found!!!")
endif()

if(${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_BINARY_DIR})
message(FATAL_ERROR "CMake generation is not allowed within source directory!! Please set a build folder with '-B'!!")
Expand Down Expand Up @@ -57,8 +58,9 @@ set(CMAKE_CXX_FLAGS_DEBUG "-O0 -g -DDEBUG=DEBUG")
set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O3 -g -DDEBUG=DEBUG")
set(CMAKE_CXX_FLAGS_CI "-O3 -DDEBUG=DEBUG")

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

# Set default values for variables/options
set(UMD_HOME "${CMAKE_SOURCE_DIR}/tt_metal/third_party/umd")
Expand Down
2 changes: 1 addition & 1 deletion INSTALLING.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Note the current compatability matrix:
sudo apt update
sudo apt install software-properties-common=0.99.9.12 build-essential=12.8ubuntu1.1 python3.8-venv=3.8.10-0ubuntu1~20.04.9 libgoogle-glog-dev=0.4.0-1build1 libyaml-cpp-dev=0.6.2-4ubuntu1 libboost-all-dev=1.71.0.0ubuntu2 libsndfile1=1.0.28-7ubuntu0.2 libhwloc-dev graphviz

# Install Clang-17: Recommended to use Clang-17 as that's what is officially supported and tested on CI.
# Install Clang-17 for C++20 support!!
wget https://apt.llvm.org/llvm.sh
chmod u+x llvm.sh
sudo ./llvm.sh 17
Expand Down
5 changes: 4 additions & 1 deletion build_metal.sh
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ Example:
<run a pytest> # <- this test ran in Release config
ninja install -C build_debug # <- install Debug pybinds
<run a pytest> # <- this test ran in Debug config
NOTE ON DEBUGGING!:
GDB/LLDB is not stable right now. Recommend to use GCC11 or higher for debugging or Clang-17 with GDB 14+
'

set -eo pipefail
Expand Down Expand Up @@ -94,7 +97,7 @@ else
fi

echo "Building tt-metal"
cmake_args="-B build -G Ninja -DCMAKE_CXX_COMPILER=clang++-17 -DCMAKE_EXPORT_COMPILE_COMMANDS=$export_compile_commands"
cmake_args="-B build -G Ninja -DCMAKE_EXPORT_COMPILE_COMMANDS=$export_compile_commands"

if [ "$enable_ccache" = "ON" ]; then
cmake_args="$cmake_args -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache"
Expand Down
14 changes: 11 additions & 3 deletions cmake/macros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ macro(CHECK_COMPILERS)
"\n Set env variable CXX=clang++-17"
"\n Check top level CMakeLists and uncomment some lines\n"
)
if(CMAKE_CXX_COMPILER_VERSION GREATER_EQUAL "10.0.0")
if(CMAKE_CXX_COMPILER_VERSION GREATER_EQUAL "11.0.0")
message(WARNING "Anything after GCC-9 has not been thoroughly tested!")
else()
message(FATAL_ERROR "Any version lower than GCC-11 will not support necessary C++20 features")
endif()
else()
message(FATAL_ERROR "Compiler is not GCC or Clang")
Expand All @@ -23,8 +25,14 @@ endmacro()
macro(CHECK_COMPILER_WARNINGS)
if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
target_compile_options(compiler_warnings INTERFACE
-Wsometimes-uninitialized -Wno-c++11-narrowing -Wno-c++20-extensions -Wno-c++23-extensions -Wno-error=local-type-template-args
-Wno-delete-non-abstract-non-virtual-dtor -Wno-c99-designator -Wno-shift-op-parentheses -Wno-non-c-typedef-for-linkage)
-Wsometimes-uninitialized -Wno-c++11-narrowing -Wno-c++23-extensions -Wno-error=local-type-template-args
-Wno-delete-non-abstract-non-virtual-dtor -Wno-c99-designator -Wno-shift-op-parentheses -Wno-non-c-typedef-for-linkage
-Wno-deprecated-this-capture -Wno-deprecated-volatile -Wno-deprecated-builtins -Wno-deprecated-declarations # -> extra C++20 build flags
)
# -Wsometimes-uninitialized will override the -Wuninitialized added before
else() # using GCC-11 or higher
target_compile_options(compiler_warnings INTERFACE
-Wno-deprecated -Wno-attributes # <-- C++20 warning flags
)
endif()
endmacro()
2 changes: 1 addition & 1 deletion cmake/umd_device.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ if($ENV{ENABLE_TRACY})
endif()

# MUST have the RPATH set, or else can't find the tracy lib
set(LDFLAGS_ "-L${CMAKE_BINARY_DIR}/lib -Wl,-rpath,${CMAKE_BINARY_DIR}/lib ${CONFIG_LDFLAGS} -ldl -lz -lboost_thread -lboost_filesystem -lboost_system -lboost_regex -lpthread -latomic -lhwloc -lstdc++")
set(LDFLAGS_ "-L${CMAKE_BINARY_DIR}/lib -L/usr/local/lib -Wl,-rpath,${CMAKE_BINARY_DIR}/lib -Wl,-rpath,/usr/local/lib ${CONFIG_LDFLAGS} -ldl -lz -lboost_thread -lboost_filesystem -lboost_system -lboost_regex -lpthread -latomic -lhwloc -lstdc++")
set(SHARED_LIB_FLAGS_ "-shared -fPIC")
set(STATIC_LIB_FLAGS_ "-fPIC")

Expand Down
2 changes: 1 addition & 1 deletion scripts/build_scripts/build_with_profiler_opt.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ if [[ -z "$ARCH_NAME" ]]; then
exit 1
fi

ENABLE_TRACY=1 ENABLE_PROFILER=1 cmake -B build -G Ninja -DCMAKE_CXX_COMPILER=clang++-17
ENABLE_TRACY=1 ENABLE_PROFILER=1 cmake -B build -G Ninja

if [[ $1 == "NO_CLEAN" ]]; then
cmake --build build
Expand Down
4 changes: 2 additions & 2 deletions tt_eager/tensor/tensor_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -799,9 +799,9 @@ inline void print_trailing_comma(std::ostream& ss, std::size_t index, std::size_
template <typename T>
inline void print_datum(std::ostream& ss, T datum) {
if (std::is_integral<T>::value) {
ss << fmt::format("{:5}", datum);
ss << std::setw(5) << datum;
} else {
ss << fmt::format("{:8.5f}", datum);
ss << std::fixed << std::setw(8) << std::setprecision(5) << datum;
}
}

Expand Down
2 changes: 2 additions & 0 deletions tt_eager/tensor/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ static_assert(
struct OwnedStorage {
OwnedBuffer buffer;
OwnedStorage() = default;
OwnedStorage(OwnedBuffer buffer_) : buffer(std::move(buffer_)) {}

static constexpr auto attribute_names = std::make_tuple();
const auto attribute_values() const { return std::make_tuple(); }
Expand All @@ -288,6 +289,7 @@ using DeviceBuffer = std::shared_ptr<Buffer>;
struct DeviceStorage {
DeviceBuffer buffer;
DeviceStorage() = default;
DeviceStorage(DeviceBuffer buffer_) : buffer(std::move(buffer_)) {}

const MemoryConfig memory_config() const {
if (this->buffer.get() == nullptr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ static inline operation::ProgramWithCallbacks create_qkv_separate(const Tensor &
UpdateDynamicCircularBufferAddress(program, cb_out2_id, *out2_buffer);
};

return {std::move(program), .override_runtime_arguments_callback = override_runtime_args_callback};
return {.program = std::move(program), .override_runtime_arguments_callback = override_runtime_args_callback};
}

namespace tt {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ static inline operation::ProgramWithCallbacks create_heads_combined_qkv_sharded(
UpdateDynamicCircularBufferAddress(program, cb_out2_id, *out2_buffer);
};

return {std::move(program), .override_runtime_arguments_callback = override_runtime_args_callback};
return {.program = std::move(program), .override_runtime_arguments_callback = override_runtime_args_callback};
}

namespace tt {
Expand Down
6 changes: 3 additions & 3 deletions tt_eager/tt_lib/csrc/tt_lib_bindings_tensor_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ void bind_op_with_mem_config(py::module_ &module, std::string op_name, Func &&f,
template <bool fused_activations = true, bool mem_config_arg = true, bool dtype_arg = true, bool opt_output_arg = true, typename Func>
void bind_binary_op(py::module_ &module, std::string op_name, Func &&f, std::string op_desc) {
std::vector<std::string> arg_name = {"input_a", "input_b"};
op_desc = fmt::format(op_desc, arg_name[0], arg_name[1]);
op_desc = fmt::format(fmt::runtime(op_desc), arg_name[0], arg_name[1]);

std::string docstring = fmt::format(R"doc(
{0}
Expand Down Expand Up @@ -156,7 +156,7 @@ void bind_binary_op(py::module_ &module, std::string op_name, Func &&f, std::str
template <bool mem_config_arg = true, bool dtype_arg = false, typename Func>
void bind_unary_op(py::module_ &module, std::string op_name, Func &&f, std::string op_desc) {
const std::string tensor_name = "input";
op_desc = fmt::format(op_desc, tensor_name);
op_desc = fmt::format(fmt::runtime(op_desc), tensor_name);
std::string docstring = fmt::format(R"doc(
{0}
Expand All @@ -178,7 +178,7 @@ template <bool mem_config_arg = true, bool dtype_arg = false, typename Func, typ
void bind_unary_op_with_param(py::module_ &module, std::string op_name, Func &&f, PyArg param, std::string op_desc, std::string param_desc) {
const std::string tensor_name = "input";
std::string param_name = std::string(param.name);
op_desc = fmt::format(op_desc, tensor_name, param_name);
op_desc = fmt::format(fmt::runtime(op_desc), tensor_name, param_name);
const std::string required_param = std::is_same_v<py::arg_v, PyArg> ? "No" : "Yes";
std::string docstring = fmt::format(R"doc(
{0}
Expand Down
6 changes: 3 additions & 3 deletions tt_metal/common/assert.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,19 +103,19 @@ void tt_assert_message(std::ostream& os, Ts const&... ts) {
fmt += "{} ";
}
log_fatal(fmt.c_str(), ts...);
os << fmt::format(fmt, ts...);
((os << fmt::format("{} ", ts)), ...);
os << std::endl;
}

template <typename... Ts>
void tt_assert_message(std::ostream& os, const char* t, Ts const&... ts) {
os << fmt::format(t, ts...);
os << fmt::format(fmt::runtime(t), ts...);
os << std::endl;
}

template <typename... Ts>
void tt_assert_message(std::ostream& os, const std::string& t, Ts const&... ts) {
os << fmt::format(t, ts...);
os << fmt::format(fmt::runtime(t), ts...);
os << std::endl;
}

Expand Down
15 changes: 15 additions & 0 deletions tt_metal/common/utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,21 @@ namespace utils
// instead of letting the program terminate.
class ThreadManager {
public:
// c++20 fix for -> error: implicit capture of 'this' with a capture default of '=' is deprecated [-Werror,-Wdeprecated-this-capture]
// not tested at all !!!
// template <typename Func, typename... Args>
// void start(Func&& func, Args&&... args) {
// auto args_tuple = std::make_tuple(std::forward<Args>(args)...);
// threads.emplace_back(std::thread([this, func=std::forward<Func>(func), args_tuple]() mutable{
// try {
// std::apply(func, args_tuple);
// } catch (...) {
// std::lock_guard<std::mutex> lock(exceptionMutex);
// exceptions.push_back(std::current_exception());
// }
// }));
// }

template <typename Func, typename... Args>
void start(Func&& func, Args&&... args) {
threads.emplace_back(std::thread([=]() {
Expand Down
1 change: 0 additions & 1 deletion tt_metal/impl/buffers/buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "tt_metal/hostdevcommon/common_values.hpp"
#include "tt_metal/impl/allocator/allocator.hpp"
#include "tt_metal/impl/device/device.hpp"
#include "tt_metal/tt_stl/stacktrace.hpp"

namespace tt {

Expand Down

0 comments on commit 43ad09f

Please sign in to comment.