[WIP] Taskgraphs #886
[WIP] Taskgraphs #886
Conversation
Co-authored-by: Swaroop Pophale <[email protected]>
|
@spophale yeah I missed that optionally
I'll go again through the tests and update these asserts EDIT 11/25/2025 -- just pushed, see changes bellow |
The spec says: '[...] Execution of the structured block associated with the construct, while optionally creating a taskgraph record of all encountered replayable constructs and the sequence in which they are encountered; or [...]' Tests now check that the structured block executed at least once, and no more than 'N' times in iterative taskgraph calls
|
Note for V&V meeting: start at taskgraph parallel test on 01/15. |
Co-authored-by: Swaroop Pophale <[email protected]>
Co-authored-by: Swaroop Pophale <[email protected]>
| } | ||
| } | ||
| } | ||
| OMPVV_TEST_AND_SET_VERBOSE(errors, !(0 < x && x <= N)); |
There was a problem hiding this comment.
OpenMP V6.0, Section 14.3 taskgraph Construct, page 436, line 12-15 says the following:
An implementation may create a finalized taskgraph record prior to the first execution of the taskgraph region, if it can guarantee that the contents of the record would match the record that would have been created during an execution of the region. In this case, a replay execution of that taskgraph record may occur upon first encountering the taskgraph construct.
Does this mean that it is possible ++x is not executed at all? If so, error checking part for x should be changed in all tests in this PR.
There was a problem hiding this comment.
I see the same wording in TR14, pg 456.
There was a problem hiding this comment.
You are right. I believe the intent here was to allow compilers to statically generate a taskgraph without ever executing its structured block.
Every tests may be impacted by that comment, as I missed these lines of the spec.
I guess we can update tests whether
- (1) with
x <= 0rather thanx < 0(x == 0 implying it got created prior to the first execution) - (2) or declaring taskgraphs in a way that they cannot be deduced prior to its first execution (for instance, spawning tasks conditionally:
# pragma omp taskgraph
{
++x;
if (external_func())
{
# pragma omp task
{}
}
}Any opinions on that? I lazily went for (1) given that implementing (2) reliably may not be that trivial/portable.
| // testTaskgraphParallel(): | ||
| // N times, have 'nthreads' threads call the same taskgraph construct | ||
| // | ||
| // ensure the structured block executed 1 to N times, |
There was a problem hiding this comment.
Although binding thread set is all threads on the current device, it is not clear from the spec as to when the taskgraph created by thread 0 is made visible to other threads.
There was a problem hiding this comment.
I agree, and it sounds hardly usable to me (having multiple threads concurrently trying to create/replay the same taskgraph record). Keeping that unresolved until further clarifications are made.
| @@ -0,0 +1,58 @@ | |||
| //===-- test_taskgraph.c ------------------------------------------------===// | |||
There was a problem hiding this comment.
This test could replace the first test "test_taskgraph.c". The test_taskgraph_replayable" should be reserved for testing the replayable clause.
There was a problem hiding this comment.
The intent of "test_taskgraph.c" was to check compiler support for the construct (with no semantical considerations). "test_taskgraph.c" was actually wrong in the case where the taskgraph record is created prior to the first region's execution, given #886 (comment)
I updated it. We could also remove it, I do not mind
| } | ||
| } | ||
| } | ||
| OMPVV_TEST_AND_SET_VERBOSE(errors, !(0 < x && x <= N)); |
There was a problem hiding this comment.
I see the same wording in TR14, pg 456.
|
@rpereira-dev , thank you for putting in the time and effort to craft these tests. Please see the review comments and commit changes that seem appropriate. We will re-review the tests once you are done with an update. |
Co-authored-by: Swaroop Pophale <[email protected]>
Co-authored-by: Swaroop Pophale <[email protected]>
Co-authored-by: Swaroop Pophale <[email protected]>
…any semantical consideration
…N*nthreads); or created prior to first region encounter (then x == 0)
…at least once: it may not be the case if it created prior to its first encounter by the compiler
Alright thanks for the feedback. Sorry for the few misunderstanding about taskgraphs specs on my side. I left unresolved the few revisions that needs additional discussions. |
3 participants
First draft for 6.0 taskgraphs.
It is not yet ready, just opening the PR so we have a place to chat about it.
I suggest we first agree on C tests, and then convert them to Fortran.