[Harvesting] simulated_harvesting_masks passed to tt_SocDescriptor (#440

)

### Issue
Related to #439 

### Description
First of the changes preparing removal of harvesting logic inside
Cluster.
perform_harvesting, and simulated_harvesting_masks arguments are now
added to affect tt_SocDescriptor.
Had to remove logic converting harvesting_mask from physical layout
coords to logical coords out of the CoordinateManager constructor.

### List of the changes
- perform_harvesting now affects harvesting in tt_SocDescriptor
- simulated_harvesting_masks are now used for harvesting in
tt_SocDescriptor
- get_tensix_harvesting_mask is a helper function which does both of
these
- shuffle_tensix_harvesting_mask is now a static public function, which
should be called prior to calling CoordinateManager
- CoordinateManager now accepts harvesting in logical coords.
- Due to deprecated get_harvesting_masks_for_soc_descriptors, there is
an additional shuffle_tensix_harvesting_mask_to_noc0_coords which does
the required translation.
- tt_SocDescriptor removed copy constructor, since it is already a
default one.
- Other functions changed accordingly to pass this arguments

### Testing
- Changed call to get_harvesting_masks_for_soc_descriptors, so it is
tested with the new flow, through harvesting provided by socdescriptor
class
- Added HarvestingShuffle tests in all arch tests

### API Changes
There are no API changes in this PR.

device/api/umd/device/cluster.h

@@ -863,8 +863,17 @@ class Cluster : public tt_device {
     void wait_for_connected_non_mmio_flush(chip_id_t chip_id);
     std::unique_ptr<Chip> construct_chip_from_cluster(
         chip_id_t chip_id, tt_ClusterDescriptor* cluster_desc, tt_SocDescriptor& soc_desc);
-    std::unique_ptr<Chip> construct_chip_from_cluster(chip_id_t logical_device_id, tt_ClusterDescriptor* cluster_desc);
+    std::unique_ptr<Chip> construct_chip_from_cluster(
+        chip_id_t logical_device_id,
+        tt_ClusterDescriptor* cluster_desc,
+        bool perform_harvesting,
+        std::unordered_map<chip_id_t, uint32_t>& simulated_harvesting_masks);
     void add_chip(chip_id_t chip_id, std::unique_ptr<Chip> chip);
+    uint32_t get_tensix_harvesting_mask(
+        chip_id_t chip_id,
+        tt_ClusterDescriptor* cluster_desc,
+        bool perform_harvesting,
+        std::unordered_map<chip_id_t, uint32_t>& simulated_harvesting_masks);
     void construct_cluster(
         const uint32_t& num_host_mem_ch_per_mmio_device,
         const bool skip_driver_allocs,

device/api/umd/device/coordinate_manager.h

@@ -42,6 +42,13 @@ class CoordinateManager {
 
     static std::vector<size_t> get_harvested_indices(const size_t harvesting_mask);
 
+    // Harvesting mask is reported by hardware in the order of physical layout. This function returns a more suitable
+    // representation in logical order: Bit 0 being set means the first row in NOC0 coords is harvested.
+    static uint32_t shuffle_tensix_harvesting_mask(tt::ARCH arch, uint32_t tensix_harvesting_physical_layout);
+    // TODO: This function should be removed once the corresponding API is removed from Cluster.
+    static uint32_t shuffle_tensix_harvesting_mask_to_noc0_coords(
+        tt::ARCH arch, uint32_t tensix_harvesting_logical_layout);
+
     CoordinateManager(CoordinateManager& other) = default;
 
     tt::umd::CoreCoord translate_coord_to(const tt::umd::CoreCoord core_coord, const CoordSystem coord_system);
@@ -91,8 +98,6 @@ class CoordinateManager {
 
     virtual void assert_coordinate_manager_constructor();
 
-    virtual void shuffle_tensix_harvesting_mask(const std::vector<uint32_t>& harvesting_locations);
-
     virtual void translate_tensix_coords();
     virtual void translate_dram_coords();
     virtual void translate_eth_coords();

device/api/umd/device/tt_soc_descriptor.h

@@ -55,40 +55,6 @@ class tt_SocDescriptor {
         const size_t dram_harvesting_mask = 0,
         const size_t eth_harvesting_mask = 0);
 
-    // Copy constructor
-    tt_SocDescriptor(const tt_SocDescriptor &other) :
-        arch(other.arch),
-        grid_size(other.grid_size),
-        worker_grid_size(other.worker_grid_size),
-        cores(other.cores),
-        arc_cores(other.arc_cores),
-        workers(other.workers),
-        harvested_workers(other.harvested_workers),
-        pcie_cores(other.pcie_cores),
-        worker_log_to_routing_x(other.worker_log_to_routing_x),
-        worker_log_to_routing_y(other.worker_log_to_routing_y),
-        routing_x_to_worker_x(other.routing_x_to_worker_x),
-        routing_y_to_worker_y(other.routing_y_to_worker_y),
-        dram_cores(other.dram_cores),
-        dram_core_channel_map(other.dram_core_channel_map),
-        ethernet_cores(other.ethernet_cores),
-        ethernet_core_channel_map(other.ethernet_core_channel_map),
-        trisc_sizes(other.trisc_sizes),
-        device_descriptor_file_path(other.device_descriptor_file_path),
-        overlay_version(other.overlay_version),
-        unpacker_version(other.unpacker_version),
-        dst_size_alignment(other.dst_size_alignment),
-        packer_version(other.packer_version),
-        worker_l1_size(other.worker_l1_size),
-        eth_l1_size(other.eth_l1_size),
-        noc_translation_id_enabled(other.noc_translation_id_enabled),
-        dram_bank_size(other.dram_bank_size),
-        coordinate_manager(other.coordinate_manager),
-        cores_map(other.cores_map),
-        grid_size_map(other.grid_size_map),
-        harvested_cores_map(other.harvested_cores_map),
-        harvested_grid_size_map(other.harvested_grid_size_map) {}
-
     // CoreCoord conversions.
     tt::umd::CoreCoord translate_coord_to(const tt::umd::CoreCoord core_coord, const CoordSystem coord_system) const;
 
@@ -140,6 +106,7 @@ class tt_SocDescriptor {
     int eth_l1_size;
     bool noc_translation_id_enabled;
     uint64_t dram_bank_size;
+    uint32_t tensix_harvesting_mask;
 
 private:
     void create_coordinate_manager(

device/blackhole/blackhole_coordinate_manager.cpp

@@ -37,7 +37,6 @@ BlackholeCoordinateManager::BlackholeCoordinateManager(
         arc_cores,
         pcie_grid_size,
         pcie_cores) {
-    this->shuffle_tensix_harvesting_mask(blackhole::HARVESTING_NOC_LOCATIONS);
     initialize();
 }
 

device/cluster.cpp

@@ -267,14 +267,13 @@ void Cluster::create_device(
 bool Cluster::using_harvested_soc_descriptors() { return perform_harvesting_on_sdesc && performed_harvesting; }
 
 std::unordered_map<chip_id_t, uint32_t> Cluster::get_harvesting_masks_for_soc_descriptors() {
-    if (using_harvested_soc_descriptors()) {
-        return harvested_rows_per_target;
-    }
-    std::unordered_map<chip_id_t, uint32_t> default_harvesting_masks = {};
-    for (const auto chip : all_chip_ids_) {
-        default_harvesting_masks.insert({chip, 0});
+    std::unordered_map<chip_id_t, uint32_t> harvesting_masks = {};
+    for (const auto& [chip_id, chip] : chips_) {
+        uint32_t noc0_harvesting_mask = CoordinateManager::shuffle_tensix_harvesting_mask_to_noc0_coords(
+            chip->get_soc_descriptor().arch, chip->get_soc_descriptor().tensix_harvesting_mask);
+        harvesting_masks.insert({chip_id, noc0_harvesting_mask});
     }
-    return default_harvesting_masks;
+    return harvesting_masks;
 }
 
 void Cluster::construct_cluster(
@@ -452,15 +451,16 @@ std::unique_ptr<Chip> Cluster::construct_chip_from_cluster(
     }
 }
 
-std::unique_ptr<Chip> Cluster::construct_chip_from_cluster(chip_id_t chip_id, tt_ClusterDescriptor* cluster_desc) {
+std::unique_ptr<Chip> Cluster::construct_chip_from_cluster(
+    chip_id_t chip_id,
+    tt_ClusterDescriptor* cluster_desc,
+    bool perform_harvesting,
+    std::unordered_map<chip_id_t, uint32_t>& simulated_harvesting_masks) {
     tt::ARCH arch = cluster_desc->get_arch(chip_id);
     std::string soc_desc_path = tt_SocDescriptor::get_soc_descriptor_path(arch);
-    // Note that initially soc_descriptors are not harvested, but will be harvested later if perform_harvesting is
-    // true.
-    // TODO: This should be changed, harvesting should be done in tt_socdescriptor's constructor and not as part of
-    // cluster class.
-    uint32_t tensix_harvesting_mask = cluster_desc->get_harvesting_info().at(chip_id);
-    tt_SocDescriptor soc_desc = tt_SocDescriptor(soc_desc_path, tensix_harvesting_mask /*, harvesting_info*/);
+    uint32_t tensix_harvesting_mask =
+        get_tensix_harvesting_mask(chip_id, cluster_desc, perform_harvesting, simulated_harvesting_masks);
+    tt_SocDescriptor soc_desc = tt_SocDescriptor(soc_desc_path, tensix_harvesting_mask);
     return construct_chip_from_cluster(chip_id, cluster_desc, soc_desc);
 }
 
@@ -478,6 +478,40 @@ void Cluster::add_chip(chip_id_t chip_id, std::unique_ptr<Chip> chip) {
     chips_.emplace(chip_id, std::move(chip));
 }
 
+uint32_t Cluster::get_tensix_harvesting_mask(
+    chip_id_t chip_id,
+    tt_ClusterDescriptor* cluster_desc,
+    bool perform_harvesting,
+    std::unordered_map<chip_id_t, uint32_t>& simulated_harvesting_masks) {
+    if (!perform_harvesting) {
+        log_info(LogSiliconDriver, "Skipping harvesting for chip {}.", chip_id);
+        return 0;
+    }
+    uint32_t tensix_harvesting_mask_physical_layout = cluster_desc->get_harvesting_info().at(chip_id);
+    uint32_t tensix_harvesting_mask = CoordinateManager::shuffle_tensix_harvesting_mask(
+        cluster_desc->get_arch(chip_id), tensix_harvesting_mask_physical_layout);
+    uint32_t simulated_harvesting_mask = (simulated_harvesting_masks.find(chip_id) != simulated_harvesting_masks.end())
+                                             ? simulated_harvesting_masks.at(chip_id)
+                                             : 0;
+    if (simulated_harvesting_mask != 0) {
+        log_info(
+            LogSiliconDriver,
+            "Adding simulated harvesting mask {} for chip {} which has real harvesting mask {}.",
+            simulated_harvesting_mask,
+            chip_id,
+            tensix_harvesting_mask);
+    }
+    log_debug(
+        LogSiliconDriver,
+        "Harvesting mask for chip {} is {} (physical layout: {}, logical: {}, simulated harvesting mask: {}).",
+        chip_id,
+        tensix_harvesting_mask | simulated_harvesting_mask,
+        tensix_harvesting_mask_physical_layout,
+        tensix_harvesting_mask,
+        simulated_harvesting_mask);
+    return tensix_harvesting_mask | simulated_harvesting_mask;
+}
+
 Cluster::Cluster(
     const uint32_t& num_host_mem_ch_per_mmio_device,
     const bool skip_driver_allocs,
@@ -487,7 +521,9 @@ Cluster::Cluster(
     cluster_desc = tt_ClusterDescriptor::create();
 
     for (auto& chip_id : cluster_desc->get_all_chips()) {
-        add_chip(chip_id, construct_chip_from_cluster(chip_id, cluster_desc.get()));
+        add_chip(
+            chip_id,
+            construct_chip_from_cluster(chip_id, cluster_desc.get(), perform_harvesting, simulated_harvesting_masks));
     }
 
     // TODO: work on removing this member altogether. Currently assumes all have the same arch.
@@ -515,7 +551,9 @@ Cluster::Cluster(
             cluster_desc->get_all_chips().find(chip_id) != cluster_desc->get_all_chips().end(),
             "Target device {} not present in current cluster!",
             chip_id);
-        add_chip(chip_id, construct_chip_from_cluster(chip_id, cluster_desc.get()));
+        add_chip(
+            chip_id,
+            construct_chip_from_cluster(chip_id, cluster_desc.get(), perform_harvesting, simulated_harvesting_masks));
     }
 
     // TODO: work on removing this member altogether. Currently assumes all have the same arch.
@@ -544,8 +582,8 @@ Cluster::Cluster(
             cluster_desc->get_all_chips().find(chip_id) != cluster_desc->get_all_chips().end(),
             "Target device {} not present in current cluster!",
             chip_id);
-
-        size_t tensix_harvesting_mask = cluster_desc->get_harvesting_info().at(chip_id);
+        size_t tensix_harvesting_mask =
+            get_tensix_harvesting_mask(chip_id, cluster_desc.get(), perform_harvesting, simulated_harvesting_masks);
         tt_SocDescriptor soc_desc = tt_SocDescriptor(sdesc_path, tensix_harvesting_mask);
         log_assert(
             cluster_desc->get_arch(chip_id) == soc_desc.arch,

device/coordinate_manager.cpp

@@ -294,24 +294,46 @@ size_t CoordinateManager::get_tensix_harvesting_mask() const { return physical_l
 
 size_t CoordinateManager::get_dram_harvesting_mask() const { return dram_harvesting_mask; }
 
-void CoordinateManager::shuffle_tensix_harvesting_mask(const std::vector<uint32_t>& harvesting_locations) {
+uint32_t CoordinateManager::shuffle_tensix_harvesting_mask(tt::ARCH arch, uint32_t tensix_harvesting_physical_layout) {
+    std::vector<uint32_t> harvesting_locations =
+        tt::umd::architecture_implementation::create(arch)->get_harvesting_noc_locations();
+
     std::vector<uint32_t> sorted_harvesting_locations = harvesting_locations;
     std::sort(sorted_harvesting_locations.begin(), sorted_harvesting_locations.end());
     size_t new_harvesting_mask = 0;
     uint32_t pos = 0;
-    while (tensix_harvesting_mask > 0) {
-        if (tensix_harvesting_mask & 1) {
+    while (tensix_harvesting_physical_layout > 0) {
+        if (tensix_harvesting_physical_layout & 1) {
             uint32_t sorted_position =
                 std::find(
                     sorted_harvesting_locations.begin(), sorted_harvesting_locations.end(), harvesting_locations[pos]) -
                 sorted_harvesting_locations.begin();
             new_harvesting_mask |= (1 << sorted_position);
         }
-        tensix_harvesting_mask >>= 1;
+        tensix_harvesting_physical_layout >>= 1;
+        pos++;
+    }
+
+    return new_harvesting_mask;
+}
+
+uint32_t CoordinateManager::shuffle_tensix_harvesting_mask_to_noc0_coords(
+    tt::ARCH arch, uint32_t tensix_harvesting_logical_layout) {
+    std::vector<uint32_t> sorted_harvesting_locations =
+        tt::umd::architecture_implementation::create(arch)->get_harvesting_noc_locations();
+
+    std::sort(sorted_harvesting_locations.begin(), sorted_harvesting_locations.end());
+    size_t new_harvesting_mask = 0;
+    uint32_t pos = 0;
+    while (tensix_harvesting_logical_layout > 0) {
+        if (tensix_harvesting_logical_layout & 1) {
+            new_harvesting_mask |= (1 << sorted_harvesting_locations[pos]);
+        }
+        tensix_harvesting_logical_layout >>= 1;
         pos++;
     }
 
-    tensix_harvesting_mask = new_harvesting_mask;
+    return new_harvesting_mask;
 }
 
 const std::vector<tt_xy_pair>& CoordinateManager::get_physical_pairs(const CoreType core_type) const {

device/grayskull/grayskull_coordinate_manager.cpp

@@ -35,7 +35,6 @@ GrayskullCoordinateManager::GrayskullCoordinateManager(
         arc_cores,
         pcie_grid_size,
         pcie_cores) {
-    this->shuffle_tensix_harvesting_mask(grayskull::HARVESTING_NOC_LOCATIONS);
     initialize();
 }
 

device/tt_soc_descriptor.cpp

@@ -222,7 +222,8 @@ tt_SocDescriptor::tt_SocDescriptor(
     std::string device_descriptor_path,
     const size_t tensix_harvesting_mask,
     const size_t dram_harvesting_mask,
-    const size_t eth_harvesting_mask) {
+    const size_t eth_harvesting_mask) :
+    tensix_harvesting_mask(tensix_harvesting_mask) {
     std::ifstream fdesc(device_descriptor_path);
     if (fdesc.fail()) {
         throw std::runtime_error(

device/wormhole/wormhole_coordinate_manager.cpp

@@ -35,7 +35,6 @@ WormholeCoordinateManager::WormholeCoordinateManager(
         arc_cores,
         pcie_grid_size,
         pcie_cores) {
-    this->shuffle_tensix_harvesting_mask(wormhole::HARVESTING_NOC_LOCATIONS);
     initialize();
 }
 

tests/api/test_core_coord_translation_bh.cpp

@@ -35,7 +35,7 @@ TEST(CoordinateManager, CoordinateManagerBlackholeNoHarvesting) {
 // the logical coordinates if the first row is harvested.
 TEST(CoordinateManager, CoordinateManagerBlackholeTopLeftCore) {
     // This is targeting first row of Tensix cores on NOC layout.
-    const size_t harvesting_mask = (1 << tt::umd::blackhole::LOGICAL_HARVESTING_LAYOUT[0]);
+    const size_t harvesting_mask = (1 << 0);
     std::shared_ptr<CoordinateManager> coordinate_manager =
         CoordinateManager::create_coordinate_manager(tt::ARCH::BLACKHOLE, harvesting_mask);
     tt_xy_pair tensix_grid_size = tt::umd::blackhole::TENSIX_GRID_SIZE;
@@ -208,8 +208,7 @@ TEST(CoordinateManager, CoordinateManagerBlackholeVirtualEqualTranslated) {
 
 // Test mapping of the coordinates for harvested DRAM bank.
 TEST(CoordinateManager, CoordinateManagerBlackholeTransltedMappingHarvested) {
-    const size_t harvesting_mask = (1 << tt::umd::blackhole::LOGICAL_HARVESTING_LAYOUT[0]) |
-                                   (1 << tt::umd::blackhole::LOGICAL_HARVESTING_LAYOUT[1]);
+    const size_t harvesting_mask = (1 << 0) | (1 << 1);
     std::shared_ptr<CoordinateManager> coordinate_manager =
         CoordinateManager::create_coordinate_manager(tt::ARCH::BLACKHOLE, harvesting_mask);
 
@@ -621,3 +620,14 @@ TEST(CoordinateManager, CoordinateManagerBlackholePhysicalLayoutTensixHarvesting
         EXPECT_EQ(coordinate_manager->get_tensix_harvesting_mask(), harvesting_mask);
     }
 }
+
+// Test whether we properly shuffle the harvesting mask based on the physical layout of the chip.
+TEST(CoordinateManager, CoordinateManagerBlackholeHarvestingShuffle) {
+    for (size_t i = 0; i < tt::umd::blackhole::LOGICAL_HARVESTING_LAYOUT.size(); i++) {
+        const size_t harvesting_mask_physical_layout = (1 << tt::umd::blackhole::LOGICAL_HARVESTING_LAYOUT[i]);
+        const size_t harvesting_mask =
+            CoordinateManager::shuffle_tensix_harvesting_mask(tt::ARCH::BLACKHOLE, harvesting_mask_physical_layout);
+
+        EXPECT_EQ(harvesting_mask, 1 << i);
+    }
+}

tests/api/test_core_coord_translation_gs.cpp

@@ -53,7 +53,7 @@ TEST(CoordinateManager, CoordinateManagerGrayskullTopLeftCore) {
 // the logical coordinates if the first row is harvested.
 TEST(CoordinateManager, CoordinateManagerGrayskullTopLeftCoreHarvesting) {
     // This is targeting first row of Tensix cores on NOC layout.
-    const size_t harvesting_mask = (1 << tt::umd::grayskull::LOGICAL_HARVESTING_LAYOUT[0]);
+    const size_t harvesting_mask = (1 << 0);
     std::shared_ptr<CoordinateManager> coordinate_manager =
         CoordinateManager::create_coordinate_manager(tt::ARCH::GRAYSKULL, harvesting_mask);
 
@@ -181,8 +181,7 @@ TEST(CoordinateManager, CoordinateManagerGrayskullLogicalVirtualMapping) {
 // Test that harvested physical coordinates map to the last row of the virtual coordinates.
 TEST(CoordinateManager, CoordinateManagerGrayskullPhysicalHarvestedMapping) {
     // Harvest first and second NOC layout row.
-    const size_t harvesting_mask = (1 << tt::umd::grayskull::LOGICAL_HARVESTING_LAYOUT[0]) |
-                                   (1 << tt::umd::grayskull::LOGICAL_HARVESTING_LAYOUT[1]);
+    const size_t harvesting_mask = (1 << 0) | (1 << 1);
     const size_t num_harvested = CoordinateManager::get_num_harvested(harvesting_mask);
     std::shared_ptr<CoordinateManager> coordinate_manager =
         CoordinateManager::create_coordinate_manager(tt::ARCH::GRAYSKULL, harvesting_mask);
@@ -207,8 +206,7 @@ TEST(CoordinateManager, CoordinateManagerGrayskullPhysicalHarvestedMapping) {
 // Test that harvested physical coordinates map to the last row of the virtual coordinates.
 TEST(CoordinateManager, CoordinateManagerGrayskullPhysicalTranslatedHarvestedMapping) {
     // Harvest first and second NOC layout row.
-    const size_t harvesting_mask = (1 << tt::umd::grayskull::LOGICAL_HARVESTING_LAYOUT[0]) |
-                                   (1 << tt::umd::grayskull::LOGICAL_HARVESTING_LAYOUT[1]);
+    const size_t harvesting_mask = (1 << 0) | (1 << 1);
     const size_t num_harvested = CoordinateManager::get_num_harvested(harvesting_mask);
     std::shared_ptr<CoordinateManager> coordinate_manager =
         CoordinateManager::create_coordinate_manager(tt::ARCH::GRAYSKULL, harvesting_mask);
@@ -325,3 +323,14 @@ TEST(CoordinateManager, CoordinateManagerGrayskullPhysicalLayoutTensixHarvesting
         EXPECT_EQ(coordinate_manager->get_tensix_harvesting_mask(), harvesting_mask);
     }
 }
+
+// Test whether we properly shuffle the harvesting mask based on the physical layout of the chip.
+TEST(CoordinateManager, CoordinateManagerGrayskullHarvestingShuffle) {
+    for (size_t i = 0; i < tt::umd::grayskull::LOGICAL_HARVESTING_LAYOUT.size(); i++) {
+        const size_t harvesting_mask_physical_layout = (1 << tt::umd::grayskull::LOGICAL_HARVESTING_LAYOUT[i]);
+        const size_t harvesting_mask =
+            CoordinateManager::shuffle_tensix_harvesting_mask(tt::ARCH::GRAYSKULL, harvesting_mask_physical_layout);
+
+        EXPECT_EQ(harvesting_mask, 1 << i);
+    }
+}