This is an archive of the discontinued LLVM Phabricator instance.

[mlir][OpenMP] Remove clauses that are not being handled
ClosedPublic

Authored by shraiysh on Feb 17 2022, 2:24 AM.

Details

Summary

This patch removes the following clauses from OpenMP Dialect:

  • private
  • firstprivate
  • lastprivate
  • shared
  • default
  • copyin
  • copyprivate

The privatization clauses are being handled in the flang frontend. The
data copying clauses are not being handled anywhere for now. Once
we have a better picture of how to handle these clauses in OpenMP
Dialect, we can add these. For the time being, removing unneeded
clauses.

For detailed discussion about this refer to Privatisation in OpenMP dialect

Diff Detail

Event Timeline

shraiysh created this revision.Feb 17 2022, 2:24 AM
shraiysh requested review of this revision.Feb 17 2022, 2:24 AM
shraiysh edited the summary of this revision. (Show Details)Feb 17 2022, 2:25 AM
shraiysh edited the summary of this revision. (Show Details)
clementval accepted this revision.Feb 17 2022, 2:44 AM

LGTM. Can you check why the pre check are failing?

This revision is now accepted and ready to land.Feb 17 2022, 2:44 AM

Can you post a reply to the privatisation thread in discourse that we are removing privatisation clauses temporarily? Will add them back when add privatisation support in MLIR.

shraiysh updated this revision to Diff 409625.Feb 17 2022, 6:28 AM

Thanks for pointing that out @clementval. I did not check-flang earlier. Handled it now.
Requesting review again because one more file has been affected.

Sure @kiranchandramohan. I will post a reply there.

Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2022, 6:28 AM
shraiysh requested review of this revision.Feb 17 2022, 6:29 AM

I think there are a few places where comments or a variable has to be removed. I am suggesting a few. Could you have a look again and remove it appropriately? You can submit it after that. I think you do not need a further review.

flang/lib/Lower/OpenMP.cpp
146

Nit: Can the defaultClauseOperand attribute be removed ?

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
533–554

Nit: Can this comment be appropriately modified?

577–580

Nit: Can the relevant unused variables be appropriately removed?

840–841

Nit: Can this comment be appropriately modified?

898

Nit: Can this comment be appropriately modified?

957

Nit: Can this comment be appropriately modified?

This revision is now accepted and ready to land.Feb 18 2022, 9:33 AM
shraiysh updated this revision to Diff 409981.Feb 18 2022, 11:21 AM
shraiysh marked 5 inline comments as done.

Thanks for the review @kiranchandramohan. Addressed comments.

This revision was automatically updated to reflect the committed changes.