Skip to content
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

Remove tile life from remaining routines #150

Merged
merged 13 commits into from
Dec 7, 2023

Conversation

neil-lindquist
Copy link
Contributor

@neil-lindquist neil-lindquist commented Dec 1, 2023

This PR removes tile life from the remaining routines. This doesn't do the big refactor to actually remove the tile life infrastructure, so that the review can focus on the logic for memory management.

A notable change is that I extended the releaseRemoteWorkspace functions to reduce multiple receive counts simultaneously, so I can get a little extra parallelism out of hbmm.

@neil-lindquist neil-lindquist marked this pull request as draft December 1, 2023 19:07
@neil-lindquist neil-lindquist changed the title Remove tile life from remaining LAPACK routines (except getrf*) (depends on #147) Remove tile life from remaining routines Dec 1, 2023
@neil-lindquist neil-lindquist marked this pull request as ready for review December 1, 2023 21:10
Copy link
Contributor

@ayarkhan ayarkhan left a comment

Choose a reason for hiding this comment

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

Question: In the newly added release tasks you sometimes use in and sometimes inout for dependencies. Given that you are usually changing (freeing) data, shouldn't inout be the norm?

#pragma omp task depend(in:row[k])
#pragma omp task depend(inout:row[k])

src/hbmm.cc Show resolved Hide resolved
@neil-lindquist
Copy link
Contributor Author

The cases I used in were routines where we used backward looking dependency (i.e., gemm[k-1]) in the compute routines. In those cases, using inout would put the release task on the critical path.

@neil-lindquist neil-lindquist merged commit d514136 into icl-utk-edu:master Dec 7, 2023
8 checks passed
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.

2 participants