Add Wall Werror (#382)

### Issue
No issue
Reported from tt_metal that they're seeing some warnings

### Description
Guard against warnings when building. Ignore unused variables/functions
not to spam with warnings, and these don't do much damage anyway.

### List of the changes
- Wall to show more warnings
- Wno-unused-variable, Wno-unused-function, Wno-unused-but-set-variable
not to spam for unused
- Werror not to allow warnings passing our CI
- Did some changes in our repo which were throwing warnings

### Testing
Current CI tests are fine

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

CMakeLists.txt

@@ -53,6 +53,18 @@ message(STATUS "UMD build type: ${CMAKE_BUILD_TYPE}")
 
 include(dependencies)
 
+# Enable all warnings and treat them as errors
+# Ignore unused variables and functions since there is no harm.
+# TODO: Re-enable sign-compare
+add_compile_options(
+    -Wall
+    -Werror
+    -Wno-unused-variable
+    -Wno-unused-function
+    -Wno-unused-but-set-variable
+    -Wno-sign-compare
+)
+
 add_subdirectory(common)
 add_subdirectory(device)
 add_subdirectory(src)

device/api/umd/device/pci_device.hpp

@@ -19,7 +19,7 @@
 #include "umd/device/types/arch.h"
 
 namespace tt::umd {
-struct semver_t;
+class semver_t;
 }  // namespace tt::umd
 
 // These are not necessarily hugepages if IOMMU is enabled.

device/api/umd/device/tt_device/tt_device.h

@@ -86,8 +86,8 @@ class TTDevice {
         std::uint64_t ordering = tt::umd::tlb_data::Relaxed);
 
 protected:
-    std::unique_ptr<architecture_implementation> architecture_impl_;
     std::unique_ptr<PCIDevice> pci_device_;
+    std::unique_ptr<architecture_implementation> architecture_impl_;
     tt::ARCH arch;
 
     bool is_hardware_hung();

device/api/umd/device/types/arch.h

@@ -57,6 +57,6 @@ static inline std::string arch_to_str(const tt::ARCH arch) {
     }
 }
 
-}  // namespace tt
-
 static inline std::ostream &operator<<(std::ostream &out, const tt::ARCH &arch) { return out << arch_to_str(arch); }
+
+}  // namespace tt

device/cluster.cpp

@@ -641,14 +641,13 @@ void Cluster::configure_active_ethernet_cores_for_mmio_device(
 
 void Cluster::populate_cores() {
     std::uint32_t count = 0;
-    for (const auto chip : soc_descriptor_per_chip) {
+    for (const auto& [chip_id, soc_desc] : soc_descriptor_per_chip) {
         workers_per_chip.insert(
-            {chip.first, std::unordered_set<tt_xy_pair>(chip.second.workers.begin(), chip.second.workers.end())});
+            {chip_id, std::unordered_set<tt_xy_pair>(soc_desc.workers.begin(), soc_desc.workers.end())});
         if (count == 0) {
-            eth_cores =
-                std::unordered_set<tt_xy_pair>(chip.second.ethernet_cores.begin(), chip.second.ethernet_cores.end());
-            for (std::uint32_t dram_idx = 0; dram_idx < chip.second.get_num_dram_channels(); dram_idx++) {
-                dram_cores.insert(chip.second.get_core_for_dram_channel(dram_idx, 0));
+            eth_cores = std::unordered_set<tt_xy_pair>(soc_desc.ethernet_cores.begin(), soc_desc.ethernet_cores.end());
+            for (std::uint32_t dram_idx = 0; dram_idx < soc_desc.get_num_dram_channels(); dram_idx++) {
+                dram_cores.insert(soc_desc.get_core_for_dram_channel(dram_idx, 0));
             }
         }
         count++;

device/coordinate_manager.cpp

@@ -159,11 +159,14 @@ CoreCoord CoordinateManager::to_physical(const CoreCoord core_coord) {
         case CoordSystem::VIRTUAL:
         case CoordSystem::TRANSLATED:
             return to_physical(to_logical(core_coord));
+        case CoordSystem::LOGICAL: {
+            auto& logical_mapping = get_logical_to_physical(core_coord.core_type);
+            return logical_mapping[{core_coord.x, core_coord.y}];
+        }
+        default:
+            throw std::runtime_error(
+                "Unexpected CoordSystem value " + std::to_string((uint8_t)core_coord.coord_system));
     }
-
-    // Coord system is surely logical.
-    auto& logical_mapping = get_logical_to_physical(core_coord.core_type);
-    return logical_mapping[{core_coord.x, core_coord.y}];
 }
 
 CoreCoord CoordinateManager::to_virtual(const CoreCoord core_coord) {
@@ -173,11 +176,14 @@ CoreCoord CoordinateManager::to_virtual(const CoreCoord core_coord) {
             return to_virtual(to_logical(core_coord));
         case CoordSystem::VIRTUAL:
             return core_coord;
+        case CoordSystem::LOGICAL: {
+            auto& logical_mapping = get_logical_to_virtual(core_coord.core_type);
+            return logical_mapping[{core_coord.x, core_coord.y}];
+        }
+        default:
+            throw std::runtime_error(
+                "Unexpected CoordSystem value " + std::to_string((uint8_t)core_coord.coord_system));
     }
-
-    // Coord system is surely logical.
-    auto& logical_mapping = get_logical_to_virtual(core_coord.core_type);
-    return logical_mapping[{core_coord.x, core_coord.y}];
 }
 
 CoreCoord CoordinateManager::to_logical(const CoreCoord core_coord) {
@@ -196,6 +202,9 @@ CoreCoord CoordinateManager::to_logical(const CoreCoord core_coord) {
             auto& translated_mapping = get_translated_to_logical(core_coord.core_type);
             return translated_mapping[{core_coord.x, core_coord.y}];
         }
+        default:
+            throw std::runtime_error(
+                "Unexpected CoordSystem value " + std::to_string((uint8_t)core_coord.coord_system));
     }
 }
 
@@ -206,11 +215,14 @@ CoreCoord CoordinateManager::to_translated(const CoreCoord core_coord) {
             return to_translated(to_logical(core_coord));
         case CoordSystem::TRANSLATED:
             return core_coord;
+        case CoordSystem::LOGICAL: {
+            auto& logical_mapping = get_logical_to_translated(core_coord.core_type);
+            return logical_mapping[{core_coord.x, core_coord.y}];
+        }
+        default:
+            throw std::runtime_error(
+                "Unexpected CoordSystem value " + std::to_string((uint8_t)core_coord.coord_system));
     }
-
-    // Coord system is surely logical.
-    auto& logical_mapping = get_logical_to_translated(core_coord.core_type);
-    return logical_mapping[{core_coord.x, core_coord.y}];
 }
 
 CoreCoord CoordinateManager::to(const CoreCoord core_coord, const CoordSystem coord_system) {
@@ -223,6 +235,9 @@ CoreCoord CoordinateManager::to(const CoreCoord core_coord, const CoordSystem co
             return to_virtual(core_coord);
         case CoordSystem::TRANSLATED:
             return to_translated(core_coord);
+        default:
+            throw std::runtime_error(
+                "Unexpected CoordSystem value " + std::to_string((uint8_t)core_coord.coord_system));
     }
 }
 
@@ -455,6 +470,10 @@ std::shared_ptr<CoordinateManager> CoordinateManager::create_coordinate_manager(
                 tt::umd::blackhole::ARC_CORES,
                 tt::umd::blackhole::PCIE_GRID_SIZE,
                 tt::umd::blackhole::PCIE_CORES);
+        case tt::ARCH::Invalid:
+            throw std::runtime_error("Invalid architecture for creating coordinate manager");
+        default:
+            throw std::runtime_error("Unexpected ARCH value " + std::to_string((int)arch));
     }
 }
 

device/mockup/tt_mockup_device.hpp

@@ -114,6 +114,5 @@ class tt_MockupDevice : public tt_device {
     std::vector<tt::ARCH> archs_in_cluster = {};
     std::set<chip_id_t> target_devices_in_cluster = {};
     std::set<chip_id_t> target_remote_chips = {};
-    tt::ARCH arch_name;
     std::shared_ptr<tt_ClusterDescriptor> cluster_descriptor;
 };

device/simulation/tt_simulation_device.cpp

@@ -25,7 +25,7 @@ flatbuffers::FlatBufferBuilder create_flatbuffer(
     flatbuffers::FlatBufferBuilder builder;
     auto data = builder.CreateVector(vec);
     auto core = tt_vcs_core(core_.x, core_.y);
-    uint64_t size = size_ == 0 ? size = vec.size() * sizeof(uint32_t) : size = size_;
+    uint64_t size = (size_ == 0 ? vec.size() * sizeof(uint32_t) : size_);
     auto device_cmd = CreateDeviceRequestResponse(builder, rw, data, &core, addr, size);
     builder.Finish(device_cmd);
     return builder;

device/tlb.cpp

@@ -7,16 +7,16 @@
 namespace tt::umd {
 
 bool tlb_data::check(const tlb_offsets &offset) const {
-    return local_offset > ((1ULL << (offset.x_end - offset.local_offset)) - 1) |
-           x_end > ((1ULL << (offset.y_end - offset.x_end)) - 1) |
-           y_end > ((1ULL << (offset.x_start - offset.y_end)) - 1) |
-           x_start > ((1ULL << (offset.y_start - offset.x_start)) - 1) |
-           y_start > ((1ULL << (offset.noc_sel - offset.y_start)) - 1) |
-           noc_sel > ((1ULL << (offset.mcast - offset.noc_sel)) - 1) |
-           mcast > ((1ULL << (offset.ordering - offset.mcast)) - 1) |
-           ordering > ((1ULL << (offset.linked - offset.ordering)) - 1) |
-           linked > ((1ULL << (offset.static_vc - offset.linked)) - 1) |
-           static_vc > ((1ULL << (offset.static_vc_end - offset.static_vc)) - 1);
+    return (local_offset > ((1ULL << (offset.x_end - offset.local_offset)) - 1)) |
+           (x_end > ((1ULL << (offset.y_end - offset.x_end)) - 1)) |
+           (y_end > ((1ULL << (offset.x_start - offset.y_end)) - 1)) |
+           (x_start > ((1ULL << (offset.y_start - offset.x_start)) - 1)) |
+           (y_start > ((1ULL << (offset.noc_sel - offset.y_start)) - 1)) |
+           (noc_sel > ((1ULL << (offset.mcast - offset.noc_sel)) - 1)) |
+           (mcast > ((1ULL << (offset.ordering - offset.mcast)) - 1)) |
+           (ordering > ((1ULL << (offset.linked - offset.ordering)) - 1)) |
+           (linked > ((1ULL << (offset.static_vc - offset.linked)) - 1)) |
+           (static_vc > ((1ULL << (offset.static_vc_end - offset.static_vc)) - 1));
 }
 
 // Helper lambda to handle bit packing