#17754: Lower Indestructible to Metal, add guidance on using static vars with non-trivial destructors

CODEOWNERS

@@ -80,7 +80,7 @@ tt_metal/hw/firmware/src/*erisc* @aliuTT @ubcheema
 tt_metal/hw/inc/ethernet/ @aliuTT @ubcheema
 tt_metal/hw/inc/wormhole/eth_l1_address_map.h @aliuTT @ubcheema
 tt_metal/third_party/tt_llk_* @rtawfik01 @ttmtrajkovic @rdjogoTT
-tt_metal/tt_stl/ @patrickroberts @ayerofieiev-tt @dmakoviichuk-tt @sminakov-tt
+tt_metal/tt_stl/ @patrickroberts @ayerofieiev-tt @dmakoviichuk-tt @sminakov-tt @omilyutin-tt
 
 sfpi/ @pgkeller
 

contributing/BestPractices.md

@@ -1,4 +1,4 @@
-# Best Practices for C++20 Repository
+# Best Practices for Contributing to TT Metal
 
 ## 1. Pass Complex Types by Const References
 
@@ -319,7 +319,7 @@ struct PadDimension {
 ```
 Motivation
 - **Bug Prevention:** Reduces the risk of bugs due to uninitialized variables.
-- **Code Safety:** Ensures that all variables have a known value, leading to safer and more predictable code.
+- **Safety:** Ensures that all variables have a known value, leading to safer and more predictable code.
 - **Ease of Review:** Simplifies code reviews by making initialization explicit.
 
 ## 16. Use Early Exit for Contract Checks
@@ -354,3 +354,54 @@ void doSomething(...) {
 - **Code Clarity:** Improves code clarity by reducing unnecessary nesting.
 - **Maintainability:** Makes the code easier to maintain by focusing on the main logic once preconditions are validated.
 - **Efficiency:** Potentially improves performance by avoiding unnecessary processing when contract conditions aren't met.
+
+## 17. Avoid `static` variables with non-trivial destructors
+### Practice
+Avoid using `static` variables with non-trivial destructors. When applicable, use `tt::stl::Indestructible<T>` to create static objects with disabled destructor.
+
+### Explanation
+Objects with static storage duration (globals, static class members, or function-local statics) live from initialization until program termination.
+
+A non-trivial destructor (i.e., one that is user-defined or virtual) may depend on the state of other objects, which might have already been destroyed by the time it is invoked. This can lead to undefined behavior or subtle bugs, especially in the multi-threaded environments.
+
+An object is considered trivially destructible if it has no custom or virtual destructor and all its bases and non-static members are also trivially destructible. Examples include: fundamental types (pointers, int, float, etc.), arrays of trivially destructible types, variables marked with `constexpr`.
+
+To ensure safe and predictable program termination, static objects should meet these criteria. If dynamic initialization is required, consider using function-local statics with `tt::stl::Indestructible<T>` that disables destruction.
+
+### Motivation
+- **Safety:** Prevents accessing objects after they have been destroyed.
+- **Maintainability:** Simplifies tracking the lifetime of objects and helps avoid errors related to destruction ordering.
+
+### Example
+**Avoid:**
+```cpp
+// Bad: Using a static object with a non-trivial destructor.
+static const std::map<int, std::string> kDeviceConfigFiles = {
+    {1, "n150.yaml"},
+    {2, "n300.yaml"},
+    {8, "t3000.yaml"}
+};
+```
+
+**Prefer:**
+```cpp
+// Option 1: Use a trivial type for static data when possible.
+constexpr std::string_view kData = "Trivial destructor! Good!";
+
+constexpr uint32_t kMaxNumberOfCommandQueues = 2;
+
+// Using array of trivially destructible types is OK.
+constexpr std::array<int, 3> kDeviceIds = {1, 2, 8};
+
+// Option 2: If dynamic initialization is required, use function-local statics with `Indestructible`.
+const auto& get_device_configs() {
+    static tt::stl::Indestructible<std::map<int, std::string_view>> configs{
+        std::map<int, std::string_view>{
+            {1, "n150.yaml"},
+            {2, "n300.yaml"},
+            {8, "t3000.yaml"}
+        }
+    };
+    return configs.get();
+}
+```

tests/tt_metal/tt_metal/stl/CMakeLists.txt

@@ -1,5 +1,6 @@
 set(UNIT_TESTS_STL_SRC
     ${CMAKE_CURRENT_SOURCE_DIR}/test_any_range.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/test_indestructible.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test_slotmap.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test_strong_type.cpp
 )

tests/tt_metal/tt_metal/stl/test_indestructible.cpp

@@ -0,0 +1,25 @@
+// SPDX-FileCopyrightText: © 2024 Tenstorrent Inc.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include "tt_metal/tt_stl/indestructible.hpp"
+
+namespace tt::stl {
+namespace {
+
+TEST(IndestructibleTest, Basic) {
+    struct DangerouslyDestructible {
+        ~DangerouslyDestructible() {
+            // Wrapping in a lambda, as `FAIL()` returns `void`.
+            []() { FAIL(); }();
+        }
+    };
+
+    Indestructible<DangerouslyDestructible> obj;
+}
+
+}  // namespace
+}  // namespace tt::stl

tt-train/sources/ttml/autograd/auto_context.cpp

@@ -26,7 +26,7 @@ uint32_t AutoContext::get_seed() const {
 }
 
 AutoContext& AutoContext::get_instance() {
-    static core::Indestructible<AutoContext> instance{};
+    static tt::stl::Indestructible<AutoContext> instance{};
     return instance.get();
 }
 std::optional<NodeId> AutoContext::add_backward_node(GradFunction&& grad_function, std::span<NodeId> links) {

tt-train/sources/ttml/autograd/auto_context.hpp

@@ -4,10 +4,10 @@
 
 #pragma once
 
+#include <indestructible.hpp>
 #include <memory>
 #include <random>
 
-#include "core/indestructible.hpp"
 #include "core/mesh_device.hpp"
 #include "graph.hpp"
 
@@ -62,7 +62,7 @@ class AutoContext {
     tt::tt_metal::distributed::MeshShape m_mesh_shape = {1, 1};
     std::unique_ptr<core::MeshDevice> m_device;
 
-    friend class core::Indestructible<AutoContext>;
+    friend class tt::stl::Indestructible<AutoContext>;
 };
 
 inline auto& ctx() {

tt_metal/tt_stl/indestructible.hpp

@@ -0,0 +1,51 @@
+// SPDX-FileCopyrightText: (c) 2024 Tenstorrent AI ULC
+//
+// SPDX-License-Identifier: Apache-2.0
+
+#pragma once
+
+#include <cstddef>
+#include <new>
+#include <utility>
+
+namespace tt::stl {
+
+// `Indestructible` is a wrapper around `T` that behaves like `T` but does not call the destructor of `T`.
+// This is useful for creating objects with static storage duration: `Indestructible` avoids heap allocation, provides
+// thread-safe construction, and ensures the destructor is no-op, so does not depend on any other objects.
+//
+//
+// Example usage:
+//
+// const auto& get_object() {
+//     static Indestructible<MyObject> object;
+//     return object.get();
+// }
+//
+template <typename T>
+class Indestructible {
+public:
+    template <typename... Args>
+    explicit Indestructible(Args&&... args) {
+        // Construct T in our aligned storage
+        new (&storage_) T(std::forward<Args>(args)...);
+    }
+
+    T& get() { return *std::launder(reinterpret_cast<T*>(&storage_)); }
+
+    const T& get() const { return *std::launder(reinterpret_cast<const T*>(&storage_)); }
+
+    // Disable copy and assignment
+    Indestructible(const Indestructible&) = delete;
+    Indestructible& operator=(const Indestructible&) = delete;
+
+    // Destructor does NOT call T's destructor.
+    // This leaves the object "indestructible."
+    ~Indestructible() = default;
+
+private:
+    // A buffer of std::byte with alignment of T and size of T
+    alignas(T) std::byte storage_[sizeof(T)];
+};
+
+}  // namespace tt::stl