Fix the order of callbacks related to the taskloop construct.
Add the iteration_count to work callbacks (according to the spec).
Use kmpc_omp_task() instead of kmp_omp_task() to include OMPT callbacks.
Add a testcase.
Details
- Reviewers
Hahnfeld hbae omalyshe protze.joachim jlpeyton - Commits
- rGcdaefac5bd3c: [OMPT] Fix OMPT callbacks for the taskloop construct and add testcase
rOMP338146: [OMPT] Fix OMPT callbacks for the taskloop construct and add testcase
rL338146: [OMPT] Fix OMPT callbacks for the taskloop construct and add testcase
Diff Detail
- Repository
- rL LLVM
Event Timeline
runtime/src/kmp_tasking.cpp | ||
---|---|---|
3754 ↗ | (On Diff #149754) | same as below |
3910 ↗ | (On Diff #149754) | We somehow need to pass a return-address in here. So probably a new variant of __kmpc_omp_task, which takes the address and adds it to the ompt-callback invocation? |
3955 ↗ | (On Diff #149754) | Shouldn't there be st(ride) in the formula? |
runtime/src/kmp_tasking.cpp | ||
---|---|---|
3955 ↗ | (On Diff #149754) | Why not use calculated below iterations count - tc? It is computed at lines 3980 - 3995, including the check against zero. |
runtime/src/kmp_tasking.cpp | ||
---|---|---|
3955 ↗ | (On Diff #149754) | There is a difference between the iteration count and the task count.
tc seems to be the task count, not the iteration count. And st(ride) is not needed for the calculation of the iteration count, right? |
runtime/src/kmp_tasking.cpp | ||
---|---|---|
3955 ↗ | (On Diff #149754) | No, the tc states for "trace count" which is exactly iterations count. And the stride is important here. |
Use tc for the iteration count
runtime/src/kmp_tasking.cpp | ||
---|---|---|
3955 ↗ | (On Diff #149754) | You're right, I updated the diff to use tc. |
Can you add another loop in the test case where this makes a difference? Maybe just Andrey's example
taskloop is actually a tasking construct and explicitly no worksharing construct. So, please move the test in the task directory.
This call wasn't correctly reverted prior to commit, @gtbercea is fixing this in D50001