-
Notifications
You must be signed in to change notification settings - Fork 103
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
Replace CommandQueue in MeshDevice #17208
Labels
Comments
3 tasks
3 tasks
ayerofieiev-tt
added a commit
that referenced
this issue
Jan 29, 2025
### Ticket #17208 ### Problem description Over time `CommandQueue` stopped playing any particular function. Today it is simply proxying calls to HWCQ via creating temporary command structures and immediately routing their execution with a big switch statement, calling into hwcq methods. This adds unreasonable complexity. After the sync with @tt-asaigal @dmakoviichuk-tt @cfjchu @omilyutin-tt , we agreed to remove this class. ### What's changed Remove CommandQueue class. Redirect all usage to HWCQ. HWCommandQueue is renamed to CommandQueue Note: * command_queue.hpp was not removed. It contains definitions of commands used by hardware_command_queue, I would prefer us to clean this up in the next PR. ### Checklist - [x] [Post commit CI](https://github.com/tenstorrent/tt-metal/actions/runs/13023052350) - [x] [Blackhole Post commit](https://github.com/tenstorrent/tt-metal/actions/runs/13023279261) - [x] [T3k Frequent](https://github.com/tenstorrent/tt-metal/actions/runs/13021799389) -
7 tasks
tt-rkim
pushed a commit
that referenced
this issue
Jan 30, 2025
(#17364) ### Ticket FYI @tt-rkim Related to @ayerofieiev-tt ticket #17208 Fixes clang-tidy ### Problem description clang-tidy on CI ([link](https://github.com/tenstorrent/tt-metal/actions/runs/13039390588/job/36414797756)) failing wtih this error since commit 58f9654 ``` /home/ubuntu/actions-runner-2/_work/tt-metal/tt-metal/tt_metal/impl/dispatch/command_queue.cpp:469:34: error: the parameter 'runtime_args_ptr' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param,-warnings-as-errors] 469 | std::shared_ptr<RuntimeArgs> runtime_args_ptr, | ^ | const & 200249 warnings generated. Suppressed 200310 warnings (200248 in non-user code, 62 NOLINT). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. 1 warning treated as error ``` ### What's changed - Make this argument const ref as suggested and kicked off code-analysis job on this branch here: https://github.com/tenstorrent/tt-metal/actions/runs/13053370584 ### Checklist - [ ] Code Analyis job previously failing - [ ] Post commit CI passes - [ ] Blackhole Post commit (if applicable) - [ ] Model regression CI testing passes (if applicable) - [ ] Device performance regression CI testing passes (if applicable) - [ ] **(For models and ops writers)** Full [new models](https://github.com/tenstorrent/tt-metal/actions/workflows/full-new-models-suite.yaml) tests passes - [ ] New/Existing tests provide coverage for changes
williamlyTT
pushed a commit
that referenced
this issue
Jan 30, 2025
### Ticket #17208 ### Problem description Over time `CommandQueue` stopped playing any particular function. Today it is simply proxying calls to HWCQ via creating temporary command structures and immediately routing their execution with a big switch statement, calling into hwcq methods. This adds unreasonable complexity. After the sync with @tt-asaigal @dmakoviichuk-tt @cfjchu @omilyutin-tt , we agreed to remove this class. ### What's changed Remove CommandQueue class. Redirect all usage to HWCQ. HWCommandQueue is renamed to CommandQueue Note: * command_queue.hpp was not removed. It contains definitions of commands used by hardware_command_queue, I would prefer us to clean this up in the next PR. ### Checklist - [x] [Post commit CI](https://github.com/tenstorrent/tt-metal/actions/runs/13023052350) - [x] [Blackhole Post commit](https://github.com/tenstorrent/tt-metal/actions/runs/13023279261) - [x] [T3k Frequent](https://github.com/tenstorrent/tt-metal/actions/runs/13021799389) -
williamlyTT
pushed a commit
that referenced
this issue
Jan 30, 2025
(#17364) ### Ticket FYI @tt-rkim Related to @ayerofieiev-tt ticket #17208 Fixes clang-tidy ### Problem description clang-tidy on CI ([link](https://github.com/tenstorrent/tt-metal/actions/runs/13039390588/job/36414797756)) failing wtih this error since commit 58f9654 ``` /home/ubuntu/actions-runner-2/_work/tt-metal/tt-metal/tt_metal/impl/dispatch/command_queue.cpp:469:34: error: the parameter 'runtime_args_ptr' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param,-warnings-as-errors] 469 | std::shared_ptr<RuntimeArgs> runtime_args_ptr, | ^ | const & 200249 warnings generated. Suppressed 200310 warnings (200248 in non-user code, 62 NOLINT). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. 1 warning treated as error ``` ### What's changed - Make this argument const ref as suggested and kicked off code-analysis job on this branch here: https://github.com/tenstorrent/tt-metal/actions/runs/13053370584 ### Checklist - [ ] Code Analyis job previously failing - [ ] Post commit CI passes - [ ] Blackhole Post commit (if applicable) - [ ] Model regression CI testing passes (if applicable) - [ ] Device performance regression CI testing passes (if applicable) - [ ] **(For models and ops writers)** Full [new models](https://github.com/tenstorrent/tt-metal/actions/workflows/full-new-models-suite.yaml) tests passes - [ ] New/Existing tests provide coverage for changes
yieldthought
pushed a commit
that referenced
this issue
Jan 31, 2025
### Ticket #17208 ### Problem description Over time `CommandQueue` stopped playing any particular function. Today it is simply proxying calls to HWCQ via creating temporary command structures and immediately routing their execution with a big switch statement, calling into hwcq methods. This adds unreasonable complexity. After the sync with @tt-asaigal @dmakoviichuk-tt @cfjchu @omilyutin-tt , we agreed to remove this class. ### What's changed Remove CommandQueue class. Redirect all usage to HWCQ. HWCommandQueue is renamed to CommandQueue Note: * command_queue.hpp was not removed. It contains definitions of commands used by hardware_command_queue, I would prefer us to clean this up in the next PR. ### Checklist - [x] [Post commit CI](https://github.com/tenstorrent/tt-metal/actions/runs/13023052350) - [x] [Blackhole Post commit](https://github.com/tenstorrent/tt-metal/actions/runs/13023279261) - [x] [T3k Frequent](https://github.com/tenstorrent/tt-metal/actions/runs/13021799389) -
yieldthought
pushed a commit
that referenced
this issue
Jan 31, 2025
(#17364) ### Ticket FYI @tt-rkim Related to @ayerofieiev-tt ticket #17208 Fixes clang-tidy ### Problem description clang-tidy on CI ([link](https://github.com/tenstorrent/tt-metal/actions/runs/13039390588/job/36414797756)) failing wtih this error since commit 58f9654 ``` /home/ubuntu/actions-runner-2/_work/tt-metal/tt-metal/tt_metal/impl/dispatch/command_queue.cpp:469:34: error: the parameter 'runtime_args_ptr' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param,-warnings-as-errors] 469 | std::shared_ptr<RuntimeArgs> runtime_args_ptr, | ^ | const & 200249 warnings generated. Suppressed 200310 warnings (200248 in non-user code, 62 NOLINT). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. 1 warning treated as error ``` ### What's changed - Make this argument const ref as suggested and kicked off code-analysis job on this branch here: https://github.com/tenstorrent/tt-metal/actions/runs/13053370584 ### Checklist - [ ] Code Analyis job previously failing - [ ] Post commit CI passes - [ ] Blackhole Post commit (if applicable) - [ ] Model regression CI testing passes (if applicable) - [ ] Device performance regression CI testing passes (if applicable) - [ ] **(For models and ops writers)** Full [new models](https://github.com/tenstorrent/tt-metal/actions/workflows/full-new-models-suite.yaml) tests passes - [ ] New/Existing tests provide coverage for changes
nikileshx
pushed a commit
to nikileshx/tt-metal
that referenced
this issue
Feb 3, 2025
…17219) ### Ticket tenstorrent#17208 ### Problem description Over time `CommandQueue` stopped playing any particular function. Today it is simply proxying calls to HWCQ via creating temporary command structures and immediately routing their execution with a big switch statement, calling into hwcq methods. This adds unreasonable complexity. After the sync with @tt-asaigal @dmakoviichuk-tt @cfjchu @omilyutin-tt , we agreed to remove this class. ### What's changed Remove CommandQueue class. Redirect all usage to HWCQ. HWCommandQueue is renamed to CommandQueue Note: * command_queue.hpp was not removed. It contains definitions of commands used by hardware_command_queue, I would prefer us to clean this up in the next PR. ### Checklist - [x] [Post commit CI](https://github.com/tenstorrent/tt-metal/actions/runs/13023052350) - [x] [Blackhole Post commit](https://github.com/tenstorrent/tt-metal/actions/runs/13023279261) - [x] [T3k Frequent](https://github.com/tenstorrent/tt-metal/actions/runs/13021799389) -
nikileshx
pushed a commit
to nikileshx/tt-metal
that referenced
this issue
Feb 3, 2025
(tenstorrent#17364) ### Ticket FYI @tt-rkim Related to @ayerofieiev-tt ticket tenstorrent#17208 Fixes clang-tidy ### Problem description clang-tidy on CI ([link](https://github.com/tenstorrent/tt-metal/actions/runs/13039390588/job/36414797756)) failing wtih this error since commit tenstorrent@58f9654 ``` /home/ubuntu/actions-runner-2/_work/tt-metal/tt-metal/tt_metal/impl/dispatch/command_queue.cpp:469:34: error: the parameter 'runtime_args_ptr' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param,-warnings-as-errors] 469 | std::shared_ptr<RuntimeArgs> runtime_args_ptr, | ^ | const & 200249 warnings generated. Suppressed 200310 warnings (200248 in non-user code, 62 NOLINT). Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well. 1 warning treated as error ``` ### What's changed - Make this argument const ref as suggested and kicked off code-analysis job on this branch here: https://github.com/tenstorrent/tt-metal/actions/runs/13053370584 ### Checklist - [ ] Code Analyis job previously failing - [ ] Post commit CI passes - [ ] Blackhole Post commit (if applicable) - [ ] Model regression CI testing passes (if applicable) - [ ] Device performance regression CI testing passes (if applicable) - [ ] **(For models and ops writers)** Full [new models](https://github.com/tenstorrent/tt-metal/actions/workflows/full-new-models-suite.yaml) tests passes - [ ] New/Existing tests provide coverage for changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
No description provided.
The text was updated successfully, but these errors were encountered: