[mlir][OpenMP] Translation support for taskloop construct #174386
Merged
Conversation
This file contains hidden or 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
tblah
requested review from
NimishMishra,
Thirumalai-Shaktivel,
bhandarkar-pranav,
ergawy,
kiranchandramohan and
kiranktp
tblah
force-pushed
the
users/tblah/taskloop-no-clauses
branch
from
218bcbf to
6841450
Compare
Contributor
Author
|
I'm not sure why the bot didn't post. @llvm/pr-subscribers-mlir-openmp |
Contributor
Author
|
See also #174105, which fixes one of the usual patterns for using taskloop. |
Stylie777
reviewed
Jan 6, 2026
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Follow up PR, kept separate because it effects ordinary tasks too #174588 |
Contributor
Author
|
Windows CI failure is unrelated: |
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
tblah
force-pushed
the
users/tblah/taskloop-no-clauses
branch
from
803ade3 to
912d1d2
Compare
Stylie777
reviewed
Jan 7, 2026
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
Outdated
Show resolved
Hide resolved
Contributor
Author
This was referenced Jan 7, 2026
NimishMishra
approved these changes
Jan 8, 2026
Contributor
NimishMishra
left a comment
NimishMishra
left a comment
There was a problem hiding this comment.
Thanks @kaviya2510, @Stylie777, and @tblah for all the work on this. LGTM
- Force the first 3 entries to the StructArg to be the bounds info - Ensure it will work when executing the tasks in parallel
Comments at Stylie777#3
I decided not to fix the TODO about zero iteration taskloops because this is part of a larger problem affecting similar constructs e.g. ordinary tasks with an if clause that evaluates to false.
This is important so that the private var cleanup applies the right cleanup region to the right variable.
The body of taskloop is outlined and so OutlinableOpenMPOpInterface is needed to ensure that language frontends know not to hoist allocas outside of the body of taskloop. The complication here is that taskloop is also a loop wrapper. Currently some code assumes that taskloop contains only the wrapped loop, and so there is no place to put the allocas other than in the loop body. This is obviously not good. Unfortunately LLVM does not seem to be able to hoist these allocas back out of the loop. The taskloop loop body will need to contain stack saves and restores, which unfortunately hinder some optimizations. I think it is better to land some taskloop in LLVM 22 than not at all. It will take more work to find an appropriate MLIR representation for allocas inside of outlinable loop wrappers.
This was my mistake whilst tidying something up. We can have an empty context structure at the point when this is run, and the helper is still required to add enough dummy entries to the geps array so that its dimensions match the size of the privatization decls. I've added a regression test.
tblah
force-pushed
the
users/tblah/taskloop-no-clauses
branch
from
b5f5567 to
a875bb7
Compare
ronlieb
added a commit
to ROCm/llvm-project
that referenced
this pull request
Jan 8, 2026
…vm#174386)" needs downstream merge work This reverts commit 1af1cc2.
tblah
added a commit
to tblah/llvm-project
that referenced
this pull request
Jan 8, 2026
This is a fix for the asan bot after llvm#174386 Failing bot: https://lab.llvm.org/buildbot/#/builders/24/builds/16371 This commit undoes a simplification I thought reduced copied+pasted code. I will merge it like this now to unblock the bot, and then work separately on a different way to share code between both callbacks.
tblah
added a commit
to tblah/llvm-project
that referenced
this pull request
Jan 8, 2026
This is a fix for the asan bot after llvm#174386 Failing bot: https://lab.llvm.org/buildbot/#/builders/24/builds/16371 This commit undoes a simplification I thought reduced copied+pasted code. I will merge it like this now to unblock the bot, and then work separately on a different way to share code between both callbacks.
tblah
added a commit
that referenced
this pull request
Jan 8, 2026
…174983) This is a fix for the asan bot after #174386 Failing bot: https://lab.llvm.org/buildbot/#/builders/24/builds/16371 This commit undoes a simplification I thought reduced copied+pasted code. I will merge it like this now to unblock the bot, and then work separately on a different way to share code between both callbacks.
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 8, 2026
…Callback (#174983) This is a fix for the asan bot after llvm/llvm-project#174386 Failing bot: https://lab.llvm.org/buildbot/#/builders/24/builds/16371 This commit undoes a simplification I thought reduced copied+pasted code. I will merge it like this now to unblock the bot, and then work separately on a different way to share code between both callbacks.
navaneethshan
pushed a commit
to qualcomm/cpullvm-toolchain
that referenced
this pull request
Jan 8, 2026
…174983) This is a fix for the asan bot after llvm/llvm-project#174386 Failing bot: https://lab.llvm.org/buildbot/#/builders/24/builds/16371 This commit undoes a simplification I thought reduced copied+pasted code. I will merge it like this now to unblock the bot, and then work separately on a different way to share code between both callbacks.
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.
This PR replaces #166903
This implements translation for taskloop, along with DSA clauses. Other clauses will follow immediately after this is merged.
This patch was collaborative work by myself, @kaviya2510, and @Stylie777. I’ve left the commits unsquashed to make authorship clear. My only changes to other author’s commits are to rebase and run clang-format.
The taskloop implementation in the runtime works roughly like this: if the number of loop iterations to perform are more than some threshold, the current task is duplicated and both resulting tasks gets half of the loop range. This continues recursively until each task has a small enough loop range to run itself in a single thread.
This leads to two implementation complexities:
With regards to testing, most existing tests in the gfortran and fujitsu test suites require the reduction clause (not part of OpenMP 4.5). I wrote some tests of my own and was satisfied that it seems to be working.
Co-authored-by: Kaviya Rajendiran [email protected]
Co-authored-by: Jack Styles [email protected]