Upstream improvements needed for MRA on devices #298
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
devreal
force-pushed
the
buffer-scope
branch
from
ba03182 to
4de1496
Compare
|
Rebased on top of current master. I need a review on this so I can merge and make progress on other PRs. |
Similar to device scratch, we may want to specify whether the buffer should be synchronized to the device or not. This can help reduce superfluous transfers in cases where the data is overwritten on the device anyway. Signed-off-by: Joseph Schuchart <[email protected]>
Signed-off-by: Joseph Schuchart <[email protected]>
Signed-off-by: Joseph Schuchart <[email protected]>
Signed-off-by: Joseph Schuchart <[email protected]>
Also more aggressive nulling of data_out pointers Signed-off-by: Joseph Schuchart <[email protected]>
Try to defer tasks whenever possible if a conflict is detected and the type is not copyable. Only throw an exception if we run out of options (e.g., if there are two competing tasks mutating a value). Signed-off-by: Joseph Schuchart <[email protected]>
Signed-off-by: Joseph Schuchart <[email protected]>
Signed-off-by: Joseph Schuchart <[email protected]>
devreal
force-pushed
the
buffer-scope
branch
from
4de1496 to
56aca3a
Compare
therault
approved these changes
Nov 12, 2024
There was a problem hiding this comment.
LGTM...
2 questions, the second is not directly related to this PR:
- this does not change the PaRSEC dependency. Does this work with the master PaRSEC (or at least commit 58f8f3089eca which is selected in TTG master)?
- we don't have TTG tests that are compiled by default, so we need to go in the output of tests to see if these .h files compile without a problem.... Should we change that at some point?
|
Yes, this does not yet need a new ParSEC version. That will be a separate PR. The CI should compile all examples and tests and so all headers are compiled multiple times. Is that what you mean? |
|
OK, good for me.
For the CI, I think we only compile the libraries in the 'build' step, and
then in the 'test' step, we compile the tests to run them. What I mean is
that we need to go inside the 'test' step to find if there is any warning /
compilation error originating from these .h files... But again, that
shouldn't block this PR.
…On Tue, Nov 12, 2024 at 3:50 PM Joseph Schuchart ***@***.***> wrote:
Yes, this does not yet need a new ParSEC version. That will be a separate
PR.
The CI should compile all examples and tests and so all headers are
compiled multiple times. Is that what you mean?
—
Reply to this email directly, view it on GitHub
<#298 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABFEZNT5NIS6QIFHZWT5TCL2AJSYNAVCNFSM6AAAAABQCDRAK6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINZRGU2TINBQHA>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Set of changes made while implementing the first MRA kernels with device support. More to come but I wanted to get these out.
Summary:
const Tonttg::Bufferto specify that the content of the buffer is immutable.broadcastkttg::device::Streamrepresenting a device stream object.I tried to break down the changes into individual commits for easier review.