Skip to content

Commit

Permalink
Fix Metal build: put detect_arch() back v2 (#178)
Browse files Browse the repository at this point in the history
I've rushed into merging Joel's change #175
Seems like there were two detect_arch() calls which were needed. 
This time I've manually verified that tt_metal builds with UMD on this
branch.

I've also added an API test to document this usage, which should be
changed, probably to tt::umd::ClusterDescriptor::get_arch(chip_id) or
something similar

Fixes #171
  • Loading branch information
broskoTT authored Oct 17, 2024
1 parent 08eb1be commit dbb30a6
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 0 deletions.
1 change: 1 addition & 0 deletions device/tt_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ using TLB_DATA = tt::umd::tlb_data;
// TODO: Remove this - it's here for Metal backwards compatibility.
// Implementation is in tt_silicon_driver.cpp.
tt::ARCH detect_arch(int pci_device_num);
tt::ARCH detect_arch();

namespace boost::interprocess{
class named_mutex;
Expand Down
12 changes: 12 additions & 0 deletions device/tt_silicon_driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ std::string hugepage_dir = hugepage_dir_env ? hugepage_dir_env : "/dev/hugepages
// TLB size for DRAM on blackhole - 4GB
const uint64_t BH_4GB_TLB_SIZE = 4ULL * 1024 * 1024 * 1024;

// TODO: Remove in favor of cluster descriptor method, when it becomes available.
// Metal uses this function to determine the architecture of the first PCIe chip
// and then verifies that all subsequent chips are of the same architecture. It
// looks like Metal is doing this because we don't provide any other way... When
Expand All @@ -87,6 +88,17 @@ tt::ARCH detect_arch(int pci_device_num) {
return info.get_arch();
}

// TODO: Remove in favor of cluster descriptor method, when it becomes available.
// There is also a function which just wants to get any architecture, since it
// presumably already checked that all archs are the same.
tt::ARCH detect_arch() {
const auto devices_info = PCIDevice::enumerate_devices_info();
if (devices_info.empty()) {
return tt::ARCH::Invalid;
}
return devices_info.begin()->second.get_arch();
}

template <typename T>
void size_buffer_to_capacity(std::vector<T> &data_buf, std::size_t size_in_bytes) {
std::size_t target_size = 0;
Expand Down
22 changes: 22 additions & 0 deletions tests/api/test_cluster_descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
#include "device/pcie/pci_device.hpp"
#include "device/tt_cluster_descriptor.h"

// TODO: Needed for detect_arch, remove when it is part of cluster descriptor.
#include "device/tt_device.h"


std::unique_ptr<tt_ClusterDescriptor> get_cluster_desc() {

Expand Down Expand Up @@ -43,6 +46,25 @@ std::unique_ptr<tt_ClusterDescriptor> get_cluster_desc() {
return cluster_desc;
}

TEST(ApiTest, DetectArch) {
// TODO: This should be part of cluster descriptor. It is currently used like this from tt_metal.
tt::ARCH arch = detect_arch();

// Expect it to be invalid if no devices are found.
if (PCIDevice::enumerate_devices().empty()) {
EXPECT_EQ(arch, tt::ARCH::Invalid);
} else {
EXPECT_NE(arch, tt::ARCH::Invalid);

// TODO: This should be the only available API, previous call should be routed to this one to get any arch.
tt::ARCH arch2 = detect_arch(PCIDevice::enumerate_devices()[0]);
EXPECT_NE(arch2, tt::ARCH::Invalid);

// In our current setup, we expect all arch to be the same.
EXPECT_EQ(arch, arch2);
}
}

TEST(ApiClusterDescriptorTest, BasicFunctionality) {

std::unique_ptr<tt_ClusterDescriptor> cluster_desc = get_cluster_desc();
Expand Down

0 comments on commit dbb30a6

Please sign in to comment.