Skip to content

Commit

Permalink
tenstorrent#8460: Enable Clang-17
Browse files Browse the repository at this point in the history
  • Loading branch information
vtangTT committed May 24, 2024
1 parent 878eb4f commit 9c11c6f
Show file tree
Hide file tree
Showing 23 changed files with 279 additions and 203 deletions.
6 changes: 6 additions & 0 deletions .github/actions/install-metal-dev-deps/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,9 @@ runs:
cmake -DCMAKE_INSTALL_PREFIX=/usr -DBUILD_SHARED_LIBS=ON .
make
sudo make install
- name: Install Clang-17
shell: bash
run: |
wget https://apt.llvm.org/llvm.sh
chmod u+x llvm.sh
sudo ./llvm.sh 17
2 changes: 1 addition & 1 deletion .github/workflows/build-artifact.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
- uses: ./.github/actions/install-python-deps
- name: Build tt-metal and libs
run: |
cmake -B build -G Ninja
cmake -B build -G Ninja -DCMAKE_CXX_COMPILER=clang++-17
cmake --build build --target tests -- -j`nproc`
cmake --build build --target install -- -j`nproc`
- name: 'Tar files'
Expand Down
11 changes: 6 additions & 5 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ jobs:
echo "TT_METAL_HOME=$(pwd)" >> $GITHUB_ENV
- name: Build tt-metal libraries
run: |
cmake -B build -G Ninja
cmake --build build -- -j`nproc`
cmake -B build -G Ninja -DCMAKE_CXX_COMPILER=clang++-17
cmake --build build
- name: Remove unnecessary artifacts
run: |
find build/ -name *.so -or -name *.a -or -name *.o | xargs strip -d
Expand All @@ -56,6 +56,7 @@ jobs:
config: [Debug, Release, RelWithDebInfo]
arch: [grayskull, wormhole_b0, blackhole]
os: [ubuntu-20.04]
fail-fast: false
needs: build-lib
name: cmake build cpptest ${{ matrix.config }} ${{ matrix.arch }}
env:
Expand All @@ -79,6 +80,6 @@ jobs:
run: tar -xvf ttm_${{ matrix.arch }}-${{ matrix.config}}.tar
- name: Build tt-metal tests
run: |
rm -f build/CMakeCache.txt
cmake -B build -G Ninja
cmake --build build --target tests -- -j`nproc`
rm -f build/CMakeCache.txt && find build -name 'cmake_pch.hxx.pch' -exec touch {} \;
cmake -B build -G Ninja -DCMAKE_CXX_COMPILER=clang++-17
cmake --build build --target tests
21 changes: 19 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
cmake_minimum_required(VERSION 3.16)
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()

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'!!")
endif()
Expand All @@ -12,6 +25,9 @@ project(tt-metal
LANGUAGES CXX
)

include(${CMAKE_SOURCE_DIR}/cmake/macros.cmake)
CHECK_COMPILERS()

############################################################################################################################
# Find all required libraries to build
############################################################################################################################
Expand Down Expand Up @@ -68,7 +84,7 @@ set(CMAKE_INSTALL_DATAROOTDIR "${CMAKE_BINARY_DIR}/tmp/share")
############################################################################################################################
add_library(metal_common_libs INTERFACE)
target_link_libraries(metal_common_libs INTERFACE
dl z pthread atomic stdc++ # system libraries
dl z pthread atomic stdc++ # system libraries
Boost::thread Boost::filesystem Boost::system Boost::regex hwloc # hwloc has no cmake support, find_package won't find it
)

Expand All @@ -79,10 +95,11 @@ add_library(linker_flags INTERFACE)

add_library(compiler_warnings INTERFACE)
target_compile_options(compiler_warnings INTERFACE -Werror -Wdelete-non-virtual-dtor -Wreturn-type -Wswitch -Wuninitialized -Wno-unused-parameter)
CHECK_COMPILER_WARNINGS() # <- add any extra compile warning flags for building with Clang-17

add_library(compiler_flags INTERFACE)
target_link_libraries(compiler_flags INTERFACE compiler_warnings)
target_compile_options(compiler_flags INTERFACE -MMD -mavx2 -fPIC -DFMT_HEADER_ONLY -fvisibility-inlines-hidden -fno-lto)
target_compile_options(compiler_flags INTERFACE -mavx2 -fPIC -DFMT_HEADER_ONLY -fvisibility-inlines-hidden -fno-lto)

if(TT_METAL_VERSIM_DISABLED)
target_compile_options(compiler_flags INTERFACE -DTT_METAL_VERSIM_DISABLED)
Expand Down
2 changes: 1 addition & 1 deletion build_metal.sh
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ else
fi

echo "Building tt-metal"
cmake -B build -G Ninja -DCMAKE_EXPORT_COMPILE_COMMANDS=$export_compile_commands
cmake -B build -G Ninja -DCMAKE_CXX_COMPILER=clang++-17 -DCMAKE_EXPORT_COMPILE_COMMANDS=$export_compile_commands
cmake --build build --target install # <- this is a general cmake way, can also just run `ninja install -C build`

echo "Building cpp tests"
Expand Down
30 changes: 30 additions & 0 deletions cmake/macros.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@

macro(CHECK_COMPILERS)
message(STATUS "Checking compilers")
if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "17.0.0" OR CMAKE_CXX_COMPILER_VERSION GREATER_EQUAL "18.0.0")
message(WARNING "Only Clang-17 is tested right now")
endif()
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
message(WARNING
"\n Recommended to use Clang for better performance"
"\n Either pass in -DCMAKE_CXX_COMPILER=clang++-17"
"\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")
message(WARNING "Anything after GCC-9 has not been thoroughly tested!")
endif()
else()
message(FATAL_ERROR "Compiler is not GCC or Clang")
endif()
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 will override the -Wuninitialized added before
endif()
endmacro()
3 changes: 2 additions & 1 deletion scripts/build_scripts/build_with_profiler_opt.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ fi

remove_default_log_locations

ENABLE_TRACY=1 ENABLE_PROFILER=1 cmake -B build -G Ninja && cmake --build build --target clean
ENABLE_TRACY=1 ENABLE_PROFILER=1 cmake -B build -G Ninja -DCMAKE_CXX_COMPILER=clang++-17
cmake --build build --target clean
cmake --build build --target install
cmake --build build --target programming_examples
PYTHON_ENV_DIR=$(pwd)/build/python_env ./create_venv.sh
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ def get_build_env():
**os.environ.copy(),
"TT_METAL_HOME": Path(__file__).parent,
"TT_METAL_ENV": "production",
"CXX": "clang++-17",
}

# This should only run when building the wheel. Should not be running for any dev flow
Expand All @@ -129,7 +130,7 @@ def run(self) -> None:
if not build_dir.exists():
build_dir.mkdir(parents=True)

cmake_args = [f"."]
cmake_args = []

nproc = subprocess.check_output(["nproc"]).decode().strip()
build_args = [f"-j{nproc}"]
Expand Down
2 changes: 1 addition & 1 deletion tests/tt_metal/test_utils/packing.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ std::vector<PackType> pack_vector(const std::vector<ValueType>& values) {
unsigned int index = 0;
std::for_each(results.begin(), results.end(), [&](PackType& result) {
for (unsigned j = 0; j < num_values_to_pack; j++) {
result |= values[index].to_packed() << (index * ValueType::SIZEOF * CHAR_BIT);
result |= values[index].to_packed() << (j * ValueType::SIZEOF * CHAR_BIT);
index++;
}
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ int main(int argc, char **argv) {
////////////////////////////////////////////////////////////////////////////
// Parameters Setup
////////////////////////////////////////////////////////////////////////////
uint32_t input_size;
uint32_t input_size = 0;
tt::DataFormat tile_format;
uint32_t total_banks = 12;
if (df == 0) {
Expand Down
4 changes: 2 additions & 2 deletions tests/tt_metal/tt_metal/perf_microbenchmark/dispatch/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#include <cstdint>
#include <unordered_map>
#include "core_coord.h"
#include "logger.hpp"
#include "tt_metal/common/logger.hpp"
#include "tt_metal/host_api.hpp"
#include "tt_metal/detail/tt_metal.hpp"
#include "tt_metal/impl/dispatch/cq_commands.hpp"
Expand Down Expand Up @@ -338,7 +338,7 @@ inline bool DeviceData::validate_one_core(Device *device,
core_string = "PCIE";
} else {
tt::log_fatal("Logical core: {} physical core {} core type {}", logical_core, phys_core, core_type);
TT_ASSERT(0, "Core type not found");
TT_ASSERT(false, "Core type not found");
}

// Read results from device and compare to expected for this core.
Expand Down
2 changes: 2 additions & 0 deletions tt_eager/tensor/tensor_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ namespace tt_metal {

namespace tensor_impl {

TensorPrintProfile TTNN_TENSOR_PRINT_PROFILE = TensorPrintProfile::Short;

std::ostream& operator<<(std::ostream& os, const DataType& dtype) {
switch (dtype) {
case DataType::BFLOAT8_B: os << "bfloat8_b"; break;
Expand Down
2 changes: 1 addition & 1 deletion tt_eager/tensor/tensor_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ enum class TensorPrintProfile {
Full,
};

inline TensorPrintProfile TTNN_TENSOR_PRINT_PROFILE = TensorPrintProfile::Short;
extern TensorPrintProfile TTNN_TENSOR_PRINT_PROFILE;

namespace detail {

Expand Down
15 changes: 5 additions & 10 deletions tt_eager/tt_dnn/op_library/operation_history.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,13 @@ namespace tt {

namespace tt_metal {


#ifdef DEBUG

namespace operation_history {

namespace detail {

OperationHistory::~OperationHistory() {
this->dump_to_csv();
}
OperationHistory::~OperationHistory() { this->dump_to_csv(); }

void OperationHistory::append(OperationRecord&& record) {
std::scoped_lock<std::mutex> lock(op_history_mutex);
Expand Down Expand Up @@ -132,15 +129,13 @@ void OperationHistory::clear() {
this->records.clear();
}

OperationHistory OPERATION_HISTORY{};

} // namespace detail

const char* csv_file_name() {
return std::getenv("OPERATION_HISTORY_CSV");
}
const char* csv_file_name() { return std::getenv("OPERATION_HISTORY_CSV"); }

bool enabled() {
return csv_file_name() != nullptr;
}
bool enabled() { return csv_file_name() != nullptr; }

void dump_to_csv() { detail::OPERATION_HISTORY.dump_to_csv(); }
void clear() { detail::OPERATION_HISTORY.clear(); }
Expand Down
16 changes: 11 additions & 5 deletions tt_eager/tt_dnn/op_library/operation_history.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#pragma once

#include <tt_eager/tensor/tensor.hpp>

#include "tt_dnn/op_library/operation.hpp"

namespace tt {
Expand All @@ -22,10 +23,15 @@ struct TensorRecord {
const Layout layout;
const std::optional<MemoryConfig> memory_config;

static constexpr auto attribute_names = std::make_tuple("storage_type", "shape", "data_type", "layout", "memory_config");
static constexpr auto attribute_names =
std::make_tuple("storage_type", "shape", "data_type", "layout", "memory_config");
const auto attribute_values() const {
return std::make_tuple(
std::cref(this->storage_type), std::cref(this->shape), std::cref(this->data_type), std::cref(this->layout), std::cref(this->memory_config));
std::cref(this->storage_type),
std::cref(this->shape),
std::cref(this->data_type),
std::cref(this->layout),
std::cref(this->memory_config));
}
};

Expand Down Expand Up @@ -54,12 +60,12 @@ struct OperationHistory {
std::vector<OperationRecord> records;
};

inline OperationHistory OPERATION_HISTORY{};
extern OperationHistory OPERATION_HISTORY;

} // namespace detail

template<typename ... Args>
inline void append(Args&& ... args) {
template <typename... Args>
inline void append(Args&&... args) {
detail::OPERATION_HISTORY.append(std::forward<Args>(args)...);
}

Expand Down
Loading

0 comments on commit 9c11c6f

Please sign in to comment.