This is an archive of the discontinued LLVM Phabricator instance.

[OPENMP52] Initial support for doacross clause.
ClosedPublic

Authored by jyu2 on Jun 22 2023, 8:00 AM.

Details

Summary

Initial support for doacross clause in ordered directive.
This is intend to replace depend clause in ordered directive.

Diff Detail

Event Timeline

jyu2 created this revision.Jun 22 2023, 8:00 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
jyu2 requested review of this revision.Jun 22 2023, 8:00 AM
ABataev added inline comments.Jun 27 2023, 5:34 AM
clang/lib/Parse/ParseOpenMP.cpp
4424–4448

Can it be unified with depenbd clause parsing? (Maybe in a separate template function)

clang/lib/Sema/SemaOpenMP.cpp
91

DoacrossClauseMapTy? Since it is intended to handle also doacross clauses.

11296–11332

Try to avoid copy-paste. Maybe introduce templated function?

11346–11359

Same, try to avoid copy-paste

24033

Same, if possible try to unify handling with the depend clause, if possible

jyu2 updated this revision to Diff 535289.Jun 28 2023, 1:59 AM

Thanks Alex for the review. This is to address his comments.

jyu2 added a comment.Jun 28 2023, 2:01 AM

Thanks Alexey for the review.

clang/lib/Parse/ParseOpenMP.cpp
4424–4448

I don't really has an idea on how to combine this two with template function. Since depend clause in ordered is deprecated in 52, and will be removed, should we leave as this?

clang/lib/Sema/SemaOpenMP.cpp
91

Changed. Thanks.

11296–11332

Not sure how to do this part.

11346–11359

The code is for both Depend and Doacross with is really try to avoid copy-paste.

24033

ActOnOpenMPDoacrossClauseCommon is added which called in
ActOnOpenMPDependClause
and
ActOnOpenMPDoacrossClause

ABataev added inline comments.Jun 28 2023, 5:35 AM
clang/lib/Parse/ParseOpenMP.cpp
4424–4448

Even ff it will be removed in 52, it will still stay for OpenMP < 52. Would be good to try to unify it.

clang/lib/Sema/SemaOpenMP.cpp
20693–20699

Better to create clauses in ActOnDoAcross and ActOnDepend, this function better to return required data as a struct/class/bolean, etc.

20700

No need for else

jyu2 updated this revision to Diff 535601.Jun 28 2023, 7:55 PM

Thanks Alexey's review. This is address his comments.

jyu2 added a comment.Jun 28 2023, 7:57 PM

Thanks.

clang/lib/Parse/ParseOpenMP.cpp
4424–4448

OKay thanks. Changed

clang/lib/Sema/SemaOpenMP.cpp
11296–11332

I just merged code into OMPC_depend.

20693–20699

Okay I create static function instead. Thanks.

Upload full context

ABataev added inline comments.Jun 29 2023, 3:26 AM
clang/lib/Sema/SemaOpenMP.cpp
20871

Remove this new line

jyu2 updated this revision to Diff 535794.Jun 29 2023, 7:42 AM

Sorry, full diff.

This revision is now accepted and ready to land.Jun 29 2023, 8:28 AM
jyu2 closed this revision.Jun 29 2023, 12:24 PM

Checked in with rG085845a2acbe