Skip to content

Conversation

@janetsc
Copy link
Contributor

@janetsc janetsc commented Oct 17, 2022

This is the first step towards threads dedicated to different hardware resources.

Added two objects to manage compute resources - HexagonHmx and HexagonHvx. These can be created on the main thread for now. The next step will be to acquire those resources on specific threads.

Tested on HDK:
pytest tests/python/contrib/test_hexagon/test_parallel_hvx_load_vtcm.py
pytest tests/python/contrib/test_hexagon/test_parallel_hvx.py
pytest tests/python/contrib/test_hexagon/test_launcher.py
pytest tests/python/contrib/test_hexagon/test_run_unit_tests.py
pytest tests/python/contrib/test_hexagon/test_software_pipeline_async.py

cc: @csullivan, @kparzysz-quic, @JosephTheOctonaut, @adstraw

@tvm-bot
Copy link
Collaborator

tvm-bot commented Oct 17, 2022

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@janetsc janetsc marked this pull request as ready for review October 18, 2022 17:25
}

TEST_F(HexagonThreadManagerTest, pipe_overflow) {
// TODO(HWE): Create a temporary thread manager with a smaller pipe for this test
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per discussion this morning, one way around this is to split out a base class for thread management, and then a HexagonThreadManager that inherits this. Hardware acquisition only occurs from the HexagonThreadManager. We could use that base class to test pipe length, etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, would like to know the feasibility of splitting into testable base class + HexagonThreadManager before we decide whether to land this PR with this disabled test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might have some complexity when we parameterize the thread_main for each hardware resource that will be dedicated to that thread. So I'd like to postpone this until that task to acquire one hardware resource per thread.

@areusch areusch added needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it and removed needs-triage PRs or issues that need to be investigated by maintainers to find the right assignees to address it labels Oct 19, 2022
Copy link
Contributor

@csullivan csullivan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, we'll need to add -mhmx to the build but it looks like the power and acquire APIs don't require this compiler flag.

Copy link
Contributor

@adstraw adstraw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. A couple small things. Is the idea that existing testing should cover this change?

//! \brief Prevent move assignment.
HexagonHvx& operator=(HexagonHvx&&) = delete;

private:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this can be deleted. No private members / methods.

int answer{0};
const unsigned threads{6};
const unsigned pipe_size{100};
const unsigned pipe_size{1000};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might add a "getter" for the pipe size and stack size.

@csullivan csullivan merged commit 213fd76 into apache:main Oct 19, 2022
@csullivan
Copy link
Contributor

Thanks @janetsc and @adstraw! This is great addition and is now merged. Looking forward to the next steps!

@janetsc janetsc deleted the thread_hw_resources branch October 19, 2022 15:45
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 10, 2022
apache#13111)

This is the first step towards threads dedicated to different hardware resources.

Added two objects to manage compute resources - HexagonHmx and HexagonHvx. These can be created on the main thread for now. The next step will be to acquire those resources on specific threads.

* Add HexagonHvx and HexagonHmx

* Disable pipe overflow test for now
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
apache#13111)

This is the first step towards threads dedicated to different hardware resources.

Added two objects to manage compute resources - HexagonHmx and HexagonHvx. These can be created on the main thread for now. The next step will be to acquire those resources on specific threads.

* Add HexagonHvx and HexagonHmx

* Disable pipe overflow test for now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants