Page MenuHomePhabricator

[mlir][openmp] Added omp.taskloop
ClosedPublic

Authored by shraiysh on Jun 8 2022, 10:37 PM.

Details

Summary

This patch adds omp.taskloop operation to OpenMP Dialect along with
tests.

Diff Detail

Event Timeline

shraiysh created this revision.Jun 8 2022, 10:37 PM
shraiysh requested review of this revision.Jun 8 2022, 10:37 PM
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
681

Any particular reason to make this I32 and grain_size, num_tasks I64? All are positive integer expressions as per the standard.

shraiysh added inline comments.Jun 9 2022, 3:23 AM
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
681

Yes, I made it the way it is accepted in the LLVM OpenMP Runtime functions. I don't mind switching them all to an IntLikeType if you'd like me to.

Ping for review!

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
681

Please switch.

peixin requested changes to this revision.Jun 20 2022, 7:14 PM

Please double check all the descriptions, which are different from those for task construct.

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
594

task construct?

600

same as above?

605

same as above?

610

same as above?

640

I don't think this is the right description for taskloop construct.

686

The collpase clause is missed?

This revision now requires changes to proceed.Jun 20 2022, 7:14 PM
shraiysh updated this revision to Diff 438601.Jun 21 2022, 2:25 AM

Addressed comments.

peixin added inline comments.Jun 21 2022, 4:22 AM
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
686

This comment is not addressed.

shraiysh added inline comments.Jun 21 2022, 5:13 AM
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
686

Yes, about that - sorry for missing it. We already have an implementation of the collapse clause translation to LLVM IR for omp.wsloop and I don't think we are using the value of collapse attribute for the translation. It should be removed from the omp.wsloop too because the collapsed loops are already part of the wsloop operation as induction variables.

peixin accepted this revision.Jun 21 2022, 11:55 PM

LGTM

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
686

Right. The value of collape clause is equal to the number of loops in the worksharing-loop nest. I checked with OpenMPToLLVMIRTranslation.cpp and it does not use the value of collape clause. When I reread worksharing-loop operation, I found several typos. I will fix them together with the collapse clause.

This revision is now accepted and ready to land.Jun 21 2022, 11:55 PM
This revision was automatically updated to reflect the committed changes.