llk_san 🧼: sanitizer architecture #961

sstanisicTT · 2025-12-17T17:02:46Z

Ticket

LLK Sanitizer base architecture PR

Problem description

Provides the architecture for HW state sanitizer + unit test. The bindings in llk_lib will be part of a separate PR that will come after HW configure functions are unified.

What's changed

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update

Checklist

All post commit CI passes
Blackhole Post commit CI passes (if applicable)

github-actions · 2025-12-17T17:02:58Z

Thank you for your contribution! 🚀

You can run tt-metal integration tests by adding the blackhole-integration-tests and/or wormhole-integration-tests labels to this pull request.

If you want to run metal post-commit tests, you can add the metal-post-commit-tests label to this pull request.

📖 For more information, please refer to our CONTRIBUTING guide.

Copilot

Pull request overview

This PR introduces the base architecture for a hardware state sanitizer (LLK Sanitizer) that tracks and validates hardware configuration state across unpacker, math, and pack operations. The implementation provides a type-safe state tracking system with three state types (determinate, indeterminate, dontcare) and includes comprehensive unit tests.

Key Changes

Introduced state_t<T> template class with three-state semantics for tracking hardware configuration state
Implemented configuration and validation functions for unpack, math, and pack operations
Added unit tests verifying state transitions and validation logic for all three operation types

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
tt_llk_wormhole_b0/llk_lib/llk_assert.h	Added LLK_PANIC macro for inverted assertion conditions
tt_llk_common/llk_san_types.h	Core type system including state_t template and hardware state structures
tt_llk_common/llk_san_output.h	Assertion and panic macros for sanitizer validation
tt_llk_common/llk_san_impl.h	Implementation of configuration and validation functions
tt_llk_common/llk_san_extended.h	Placeholder for future extended state mask functionality
tt_llk_common/llk_san_backend.h	Backend configuration (thread index placeholder)
tt_llk_common/llk_san.h	Main API facade delegating to implementation functions
tests/sources/sanitizer/san_hw_config_test.cpp	Comprehensive unit tests for all three operation types
tests/python_tests/sanitizer/test_san_hw_config.py	Python test wrapper
tests/helpers/src/trisc.cpp	Global state variable definitions
tests/Makefile	Updated include paths and source directory search order
setup_clangd.sh	Added tt_llk_common to include paths for IDE support

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/sources/sanitizer/san_hw_config_test.cpp

Copilot · 2025-12-17T17:12:50Z

tt_llk_common/llk_san_impl.h

+    LLK_PANIC(reconfig && !state.is_configured, "panic: llk_san: user tried to reconfigure packer before configuring it");
+    LLK_PANIC(!reconfig && state.is_configured, "panic: llk_san: user tried to configure packer twice");


The panic checks in pack_hw_configure_impl are in reverse order compared to unpack_hw_configure_impl and math_hw_configure_impl. For consistency and maintainability, the order should match: check reconfigure-before-configure first (line 111), then check double-configure second (line 112).

Suggested change

LLK_PANIC(reconfig && !state.is_configured, "panic: llk_san: user tried to reconfigure packer before configuring it");

LLK_PANIC(!reconfig && state.is_configured, "panic: llk_san: user tried to configure packer twice");

LLK_PANIC(!reconfig && state.is_configured, "panic: llk_san: user tried to configure packer twice");

LLK_PANIC(reconfig && !state.is_configured, "panic: llk_san: user tried to reconfigure packer before configuring it");

tt_llk_common/llk_san_types.h

tt_llk_common/llk_san_impl.h

tt_llk_common/llk_san_types.h

tt_llk_common/llk_san_impl.h

tt_llk_common/llk_san_types.h

fvranicTT · 2025-12-17T17:44:02Z

tt_llk_common/llk_san_types.h

+        return this->template operator= <T>(rhs);
+    }
+
+    state_t& operator=(state_t&& rhs)


this should also be a noexcept.

Now that I think of it, to complete the rule of 5, you need to define a destructor. Sure, it will be defined implicitly, but since you already have the other 4, adding one more line won't hurt.

fvranicTT · 2025-12-17T17:48:15Z

tt_llk_common/llk_san_types.h

+            return *this;
+        }
+
+        state_type = rhs.state_type;


How would this work if U != T? state_t<int> and state_t<float> are different classes, they can't access each other's private members without a friend declaration.

I was sure I added a friend declaration somewhere to avoid this exact problem. Might have accidentally removed it accidentally

fvranicTT · 2025-12-17T17:49:03Z

tt_llk_common/llk_san_types.h

+};
+
+template <typename T, typename U>
+static inline bool _assert_condition(const state_t<T>& lhs, const state_t<U>& rhs)


static inline is redundant for function templates in headers, right?

TIL template implies static inline on functions. Though it makes sense

fvranicTT · 2025-12-17T17:49:31Z

tt_llk_common/llk_san_types.h

+        return *this;
+    }
+
+    bool is_determinate() const


This could be constexpr, right?

fvranicTT · 2025-12-17T17:50:25Z

tt_llk_common/llk_san_types.h

+        return state_type == state_type_t::dontcare;
+    }
+
+    const T& get_underlying() const


This could also be constexpr.

fvranicTT · 2025-12-17T18:26:16Z

tt_llk_common/llk_san_types.h

+        typename U,
+        typename = std::enable_if_t<
+            !is_state_t_v<std::decay_t<U>> && !std::is_same_v<std::decay_t<U>, dontcare_t> && !std::is_same_v<std::decay_t<U>, indeterminate_t>>>
+    constexpr state_t(U&& value) : underlying(std::forward<U>(value)), state_type(state_type_t::determinate)


Suggested change

constexpr state_t(U&& value) : underlying(std::forward<U>(value)), state_type(state_type_t::determinate)

constexpr state_t(U&& value) noexcept(std::is_nothrow_constructible_v<T, U&&>) : underlying(std::forward<U>(value)), state_type(state_type_t::determinate)

tt_llk_common/llk_san_types.h

fvranicTT · 2025-12-17T18:34:33Z

tt_llk_common/llk_san_types.h

+    template <typename U>
+    state_t& operator=(state_t<U>&& rhs)


Suggested change

template <typename U>

state_t& operator=(state_t<U>&& rhs)

template <typename U, typename = std::enable_if_t<std::is_same_v<T, U> || std::is_assignable_v<T&, U&&>>>

state_t& operator=(state_t<U>&& rhs) noexcept(std::is_nothrow_assignable_v<T&, U&&>)

Ditto. I hope I haven't made a typo somewhere...

tt_llk_common/llk_san_types.h

tests/helpers/src/trisc.cpp

tt_llk_common/llk_san_types.h

fvranicTT · 2025-12-17T19:00:14Z

tt_llk_common/llk_san_output.h

+
+#define LLK_SAN_ASSERT(lhs, rhs, message) LLK_ASSERT(llk_san::_assert_condition(lhs, rhs), message)
+
+#define LLK_SAN_PANIC(lhs, rhs, message) LLK_ASSERT(llk_san::_panic_condition(lhs, rhs), message)


I see that this is not used much in the code (one other mention in commented out section). So should we be using LLK_PANIC instead? It's easy to confuse the macros at the moment.

This define will be modified in the future to have different behavior in tt-metal so I wanted to prepare for it. I will consider possible better names to remove the confusion

I just realized that you meant should we be using LLK_PANIC instead of LLK_ASSERT for LLK_SAN_PANIC. The answer is that this is currently correct code due to how _panic_condition is implemented. I will figure out a way to refactor this so that it makes more sense logically.

Copilot AI review requested due to automatic review settings December 17, 2025 17:02

sstanisicTT requested review from amahmudTT, amokanTT, fvranicTT, ldjurovicTT, lpremovicTT, ncvetkovicTT, nvelickovicTT, rdjogoTT, rtawfik01, skotaracTT, skrsmanovicTT and vkrsmanovicTT as code owners December 17, 2025 17:02

Copilot started reviewing on behalf of sstanisicTT December 17, 2025 17:03 View session

sstanisicTT changed the title ~~Sstanisic/san arch pr~~ llk_san 🧼: sanitizer hw state tracking architecture Dec 17, 2025

Copilot AI reviewed Dec 17, 2025

View reviewed changes

fvranicTT reviewed Dec 17, 2025

View reviewed changes

tt_llk_common/llk_san_types.h Outdated Show resolved Hide resolved

fvranicTT reviewed Dec 17, 2025

View reviewed changes

tt_llk_common/llk_san_types.h Outdated Show resolved Hide resolved

fvranicTT reviewed Dec 17, 2025

View reviewed changes

tt_llk_common/llk_san_types.h Outdated Show resolved Hide resolved

fvranicTT reviewed Dec 18, 2025

View reviewed changes

sstanisicTT mentioned this pull request Dec 18, 2025

LLK sanitizer - MVP (init/config order, init/compute args equality) tenstorrent/tt-metal#28504

Open

github-actions bot added test-infra This label is used for issues, pull requests, or tasks related to the LLK testing framework wormhole labels Dec 25, 2025

sstanisicTT changed the title ~~llk_san 🧼: sanitizer hw state tracking architecture~~ llk_san 🧼: sanitizer architecture Dec 25, 2025

sstanisicTT added 3 commits December 25, 2025 13:11

llk_san: architechture

848bef1

llk_san: arch: add missing !state.is_configured

932f964

llk_san: arch: is_configured shouldn't be state_t

bccd87d

sstanisicTT added 18 commits December 25, 2025 13:11

llk_san: arch: comment out extended_state_mask

240d58e

llk_san: arch: add include dir tt_llk_common

e51344f

llk_san: arch: correct parameter types

3b062e2

llk_san: arch: respect order of construction

9babd7f

llk_san: arch: add global state

291ba9f

llk_san: arch: inverted assert in unpack operand check

9a50f88

llk_san: arch: add LLK_PANIC macro

3509efe

llk_san: arch: LLK_SAN_ASSERT, LLK_SAN_PANIC

040ae76

llk_san: arch: bug with implicit assignment operators

61264ae

llk_san: arch: (unpack|math|pack)_hw_configure_impl test

579f35f

llk_san: arch: clangd script update

433684f

llk_san: arch: fixup assertion order

1dea2e5

llk_san: arch: DONTCARE -> IGNORE

6832fe9

llk_san: arch: INDETERMINATE -> UNKNOWN

9e17777

llk_san: arch: DETERMINATE -> KNOWN

63acb35

llk_san: arch: fixup formatting

d3f5f46

llk_san: arch: add noexcepts

dbe283e

llk_san: arch: implement operation_save, operation_check

3cb0b49

sstanisicTT force-pushed the sstanisic/san-arch-pr branch from c17f3e3 to 3cb0b49 Compare December 25, 2025 13:11

llk_san: lib: add sanitizer hooks for Blackhole

303cc88

github-actions bot added the blackhole label Dec 26, 2025

sstanisicTT added 6 commits December 26, 2025 16:48

infra 🚧: move llk_assert.h to common lib

5063135

llk_san: arch: arch hw_state_t to operand_state_t

c1cd439

llk_san: lib: fixup

44f0160

llk_san: fix case N == 0 in operation save/check impl

0c4338c

llk_san: arch: add missing const qualifier

f1fb4f7

temp

48ec6f0

		LLK_PANIC(reconfig && !state.is_configured, "panic: llk_san: user tried to reconfigure packer before configuring it");
		LLK_PANIC(!reconfig && state.is_configured, "panic: llk_san: user tried to configure packer twice");

	constexpr state_t(U&& value) : underlying(std::forward<U>(value)), state_type(state_type_t::determinate)
	constexpr state_t(U&& value) noexcept(std::is_nothrow_constructible_v<T, U&&>) : underlying(std::forward<U>(value)), state_type(state_type_t::determinate)


		#define LLK_SAN_ASSERT(lhs, rhs, message) LLK_ASSERT(llk_san::_assert_condition(lhs, rhs), message)

		#define LLK_SAN_PANIC(lhs, rhs, message) LLK_ASSERT(llk_san::_panic_condition(lhs, rhs), message)

llk_san 🧼: sanitizer architecture #961

Are you sure you want to change the base?

llk_san 🧼: sanitizer architecture #961

Uh oh!

Conversation

sstanisicTT commented Dec 17, 2025

Ticket

Problem description

What's changed

Type of change

Checklist

Uh oh!

github-actions bot commented Dec 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants