-
Notifications
You must be signed in to change notification settings - Fork 22
[skip uplift] Add missing uninits for blackhole #1029
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thank you for your contribution! 🚀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds missing uninitialization functions for Blackhole architecture's Low-Level Kernel (LLK) library. The PR is part of a series (second of three) addressing issue #33825, which identifies that most init functions in the LLK leave hardware in an undefined state without corresponding cleanup. These uninit functions restore hardware registers and counters to their default states after operations complete.
Key changes:
- Added uninit functions to unpack operations (untilize, matmul, reduce, and generic A/AB unpacking) that restore address counters and configuration registers
- Added empty uninit stubs for math operations that use only transient state
- Added empty uninit stub for pack operations where default state matches initialized state
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tt_llk_blackhole/llk_lib/llk_unpack_untilize.h | Adds uninit function with conditional logic to either restore from saved GPRs or reset to default values |
| tt_llk_blackhole/llk_lib/llk_unpack_AB_reduce_custom.h | Updates uninit function to accept dimension parameters and restore untilizer state; includes commented code referencing fp32 accumulation requirements |
| tt_llk_blackhole/llk_lib/llk_unpack_AB_matmul.h | Adds uninit function to reset unpacker A and B address counters to defaults |
| tt_llk_blackhole/llk_lib/llk_unpack_AB.h | Adds uninit function to reset unpacker A and B address counters to defaults |
| tt_llk_blackhole/llk_lib/llk_unpack_A.h | Adds templated uninit function to reset unpacker counter and debug feature register |
| tt_llk_blackhole/llk_lib/llk_pack.h | Adds empty uninit stub noting pack_init already sets default state |
| tt_llk_blackhole/llk_lib/llk_math_welfords_sfpu.h | Adds empty uninit stub for Welford's algorithm SFPU operations |
| tt_llk_blackhole/llk_lib/llk_math_transpose_dest.h | Adds empty uninit stub for destination transpose operations |
| tt_llk_blackhole/llk_lib/llk_math_reduce_custom.h | Adds empty uninit stub for custom block reduce operations |
| tt_llk_blackhole/llk_lib/llk_math_reduce.h | Adds empty uninit stub for reduce operations |
| tt_llk_blackhole/llk_lib/llk_math_matmul.h | Adds empty uninit stub for matrix multiplication operations |
| tt_llk_blackhole/llk_lib/llk_math_eltwise_unary_sfpu.h | Adds empty uninit stub for unary SFPU element-wise operations |
| tt_llk_blackhole/llk_lib/llk_math_eltwise_unary_datacopy.h | Adds empty uninit stub for unary datacopy operations |
| tt_llk_blackhole/llk_lib/llk_math_eltwise_ternary_sfpu.h | Adds empty uninit stub for ternary SFPU element-wise operations |
| tt_llk_blackhole/llk_lib/llk_math_eltwise_binary_sfpu.h | Adds empty uninit stub for binary SFPU element-wise operations |
| tt_llk_blackhole/llk_lib/llk_math_eltwise_binary.h | Adds empty uninit stub for binary element-wise operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🚀 tt-metal post-commit testsBranch:
Test Results:
🔗 Links📊 Post-commit workflow: #20597970904 |
🚀 tt-metal post-commit testsBranch:
Test Results: 🔗 Links📊 Post-commit workflow: #20617860802 |
🚀 tt-metal post-commit testsBranch:
Test Results: 🔗 Links📊 Post-commit workflow: #20619527396 |
🚀 tt-metal post-commit testsBranch:
Test Results:
🔗 Links📊 Post-commit workflow: #20624129974 |
ea20f9a to
cd0f11a
Compare
🚀 tt-metal post-commit testsBranch:
Test Results:
🔗 Links📊 Post-commit workflow: #20919444135 |
### Ticket [Link to Github Issue](#33825) LLK PR: tenstorrent/tt-llk#1029 ### Problem description This PR uplifts LLK to the newest state of uninits and makes changes to unints function which are needed due to the changes in the llk API . ### What's changed LLK module hash. unpack_AB_reduce_unint explicitly sets unints args. ### Checklist - [ ] [](https://github.com/tenstorrent/tt-metal/actions/workflows/all-post-commit-workflows.yaml?query=branch:vvukomanovic/0126-bh-unints-uplift) - [ ] [](https://github.com/tenstorrent/tt-metal/actions/workflows/blackhole-post-commit.yaml?query=branch:vvukomanovic/0126-bh-unints-uplift) - [ ] [](https://github.com/tenstorrent/tt-metal/actions/workflows/tt-metal-l2-nightly.yaml?query=branch:vvukomanovic/0126-bh-unints-uplift) - [ ] New/Existing tests provide coverage for changes #### Model tests If your changes cover model-related code, you should run tests corresponding to affected models and platforms (Single card, T3K, Galaxy). "Choose your pipeline" workflows facilitate running multiple kinds of tests in a single run. Each offers `models-mandatory` and `models-extended` presets. The former includes a minimal set of tests, to be run always. The latter extends that with additional ones - use your best judgement in deciding which is the most appropriate for your PR. - [ ] [](https://github.com/tenstorrent/tt-metal/actions/workflows/pipeline-select.yaml?query=branch:vvukomanovic/0126-bh-unints-uplift) - [ ] `models-mandatory` preset (runs: [Device perf regressions](https://github.com/tenstorrent/tt-metal/actions/workflows/perf-device-models.yaml) and [Frequent model and ttnn tests](https://github.com/tenstorrent/tt-metal/actions/workflows/fast-dispatch-full-regressions-and-models.yaml)) - [ ] `models-extended` preset (runs: the mandatory tests, plus [Demo](https://github.com/tenstorrent/tt-metal/actions/workflows/single-card-demo-tests.yaml) and [Model perf](https://github.com/tenstorrent/tt-metal/actions/workflows/perf-models.yaml) tests) - [ ] other selection - specify runs - [ ] [](https://github.com/tenstorrent/tt-metal/actions/workflows/pipeline-select-t3k.yaml?query=branch:vvukomanovic/0126-bh-unints-uplift) - [ ] `models-mandatory` preset (runs: [Unit tests](https://github.com/tenstorrent/tt-metal/actions/workflows/t3000-unit-tests.yaml)) - [ ] `models-extended` preset (runs: the mandatory tests, plus [Demo](https://github.com/tenstorrent/tt-metal/actions/workflows/t3000-demo-tests.yaml) and [Model perf](https://github.com/tenstorrent/tt-metal/actions/workflows/t3000-model-perf-tests.yaml) tests) - [ ] other selection - specify runs - [ ] [](https://github.com/tenstorrent/tt-metal/actions/workflows/pipeline-select-galaxy.yaml?query=branch:vvukomanovic/0126-bh-unints-uplift) - [ ] `models-mandatory` preset (runs: [Quick tests](https://github.com/tenstorrent/tt-metal/actions/workflows/galaxy-quick.yaml)) - [ ] `models-extended` preset (runs: the mandatory tests, plus [Demo](https://github.com/tenstorrent/tt-metal/actions/workflows/galaxy-demo-tests.yaml) and [Model perf](https://github.com/tenstorrent/tt-metal/actions/workflows/galaxy-model-perf-tests.yaml) tests) - [ ] other selection - specify runs
Ticket
Github issue
Problem description
Majority of initis in the LLK are missing unints and leaving HW in random state for the next operation.
What's changed
Second PR which implements missing BH unints. This is the second out of 3 PRs (third is for Quasar).
Type of change
Checklist