This is an archive of the discontinued LLVM Phabricator instance.

[OPENMP52] Codegen support for doacross clause.
ClosedPublic

Authored by jyu2 on Jun 29 2023, 9:01 PM.

Details

Summary

This is following the ordered depend clause.
1> Using OMPClause instead OMPDependClause in emitDoacrossOrdered to hand both

OMPDependClause and OMPDoacrossClause.

2> Add new utility function emitRestoreIP which used for both clauses.

Diff Detail

Event Timeline

jyu2 created this revision.Jun 29 2023, 9:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 29 2023, 9:01 PM
jyu2 requested review of this revision.Jun 29 2023, 9:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 29 2023, 9:01 PM
jyu2 edited the summary of this revision. (Show Details)Jun 29 2023, 9:02 PM
ABataev added inline comments.Jun 30 2023, 5:58 AM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
11381–11385

I suggest to create template static function, specialized for OMPDependClause and OMPDoacrossClauseto avoid all those multiple selects.

11382–11385

Just C is better, or Cl

11383–11386
const auto *DepC = dyn_cast<OMPDependClause>(CL);
const auto *DoC = dyn_cast<OMPDoacrossClause>(CL);
11412–11413

Try to unify this code, avoid duplications

clang/lib/CodeGen/CGStmtOpenMP.cpp
5845–5846

const auto *

5873–5884

Same, try to unify the code

jyu2 updated this revision to Diff 536394.Jun 30 2023, 1:36 PM
jyu2 marked an inline comment as done.

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

clang/lib/CodeGen/CGOpenMPRuntime.cpp
11382–11385

Changed. Thanks.

11383–11386

Thanks Alexey. Changed

11412–11413

Yes thanks. Changed.

clang/lib/CodeGen/CGStmtOpenMP.cpp
5845–5846

Changed.

5873–5884

Thanks. Changed.

ABataev added inline comments.Jun 30 2023, 1:48 PM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
11401–11402

RT is needed only for loc and tid, you can preemit them in the member functions and pass here instead of emitting them here and expose private interfaces.

11418–11424

Use Cl or just C instead of CL.

clang/lib/CodeGen/CGOpenMPRuntime.h
1661–1671

Why these declarations are moved?

jyu2 updated this revision to Diff 536443.Jun 30 2023, 2:48 PM

Thanks Alexey!!!

clang/lib/CodeGen/CGOpenMPRuntime.cpp
11401–11402

Yes, Thanks. Changed.

11418–11424

Sorry. Changed.

ABataev added inline comments.Jul 3 2023, 5:29 AM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
11405–11406
  1. const auto *
  2. Can you try torework it to make it more C++-ish? Use template deduction to avoid runtime dyn_casts.

e.g. something like:

namespace {
template <typename T>
class OMPDoacrossKind {
public:
bool isSink(const T *) {return false;}
}
}
template <>
class OMPDoacrossKInd<OMPDependClause> {
public:
bool isSink(const OMPDependClause* C) {return C->getDependenceType();}
}
...

and use if (OMPDoacrossKInd<T>::isSink(C)) RTLFn = ... and same for Source

clang/lib/CodeGen/CGStmtOpenMP.cpp
5859–5868

Same, use template-based analysis instead of runtime-based.

jyu2 updated this revision to Diff 536798.Jul 3 2023, 9:29 AM
jyu2 added inline comments.
clang/lib/CodeGen/CGOpenMPRuntime.cpp
11405–11406

Good idea!!! Thank you so much. Changed.

clang/lib/CodeGen/CGStmtOpenMP.cpp
5859–5868

Thanks. Changed.

ABataev added inline comments.Jul 3 2023, 9:59 AM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
11410

Add a message, please

clang/lib/CodeGen/CGOpenMPRuntime.h
2279

Add isSOurce

clang/lib/CodeGen/CGStmtOpenMP.cpp
5860–5862

bool IsDependSource = ODK.isSource(C);

jyu2 updated this revision to Diff 536838.Jul 3 2023, 10:30 AM

Thanks Alexey for the review!!

clang/lib/CodeGen/CGOpenMPRuntime.cpp
11410

Added. Thanks for the review.

clang/lib/CodeGen/CGOpenMPRuntime.h
2279

Opps.. Sorry. Added. Thanks!!! Jennifer

clang/lib/CodeGen/CGStmtOpenMP.cpp
5860–5862

Thanks. Changed.

ABataev accepted this revision.Jul 3 2023, 12:11 PM

LG with a nit

clang/lib/CodeGen/CGOpenMPRuntime.h
2285–2297

remove parens from the expressions, they are not needed

This revision is now accepted and ready to land.Jul 3 2023, 12:11 PM
jyu2 added a comment.Jul 3 2023, 2:01 PM

LG with a nit

Thank you so much!
Jennifer

This revision was landed with ongoing or failed builds.Jul 3 2023, 3:35 PM
This revision was automatically updated to reflect the committed changes.