This is an archive of the discontinued LLVM Phabricator instance.

[MLIR][OpenMP] Added map clause support for Target
ClosedPublic

Authored by TIFitis on Mar 30 2023, 10:34 AM.

Details

Diff Detail

Event Timeline

TIFitis created this revision.Mar 30 2023, 10:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2023, 10:34 AM
TIFitis requested review of this revision.Mar 30 2023, 10:34 AM

ping for reviews. Thanks :)

LG. Couple of Nit comments.

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
815

Nit: Add a test for the verifier as well.

mlir/test/Dialect/OpenMP/ops.mlir
492

Nit: You mean map type? Add more variants.

This revision is now accepted and ready to land.Apr 11 2023, 3:38 AM
TIFitis updated this revision to Diff 512417.Apr 11 2023, 6:11 AM
TIFitis marked an inline comment as done.

Addressed reviewer comments.

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
815

Could you please let me know where to add tests for the verifier?

mlir/test/Dialect/OpenMP/ops.mlir
492

Hi, the map clause itself is optional for the target directive.

I've added more variants.

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
815

mlir/test/Dialect/OpenMP/invalid.mlir

TIFitis updated this revision to Diff 512454.Apr 11 2023, 7:43 AM
TIFitis marked an inline comment as not done.

Changed verifier to emit error instead of silent failure.
Added verifier tests.

TIFitis marked 2 inline comments as done.Apr 11 2023, 8:08 AM
TIFitis added inline comments.
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
815

I've added the verifier tests.

Let me know if the error msg is alright. Thanks :)

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
815

I think values are not capitalised. Please check the other messages and keep it consistent.

TIFitis updated this revision to Diff 512466.Apr 11 2023, 8:28 AM
TIFitis marked an inline comment as done.

Changed error message to lwoer case.

This revision was landed with ongoing or failed builds.Apr 11 2023, 8:33 AM
This revision was automatically updated to reflect the committed changes.