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
11383–11387

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

11384–11387

Just C is better, or Cl

11385–11388
const auto *DepC = dyn_cast<OMPDependClause>(CL);
const auto *DoC = dyn_cast<OMPDoacrossClause>(CL);
11414–11415

Try to unify this code, avoid duplications

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

const auto *

5871–5882

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
11384–11387

Changed. Thanks.

11385–11388

Thanks Alexey. Changed

11414–11415

Yes thanks. Changed.

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

Changed.

5871–5882

Thanks. Changed.

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

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.

11420–11426

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
11403–11404

Yes, Thanks. Changed.

11420–11426

Sorry. Changed.

ABataev added inline comments.Jul 3 2023, 5:29 AM
clang/lib/CodeGen/CGOpenMPRuntime.cpp
11407–11408
  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
11407–11408

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
11412

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
11412

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.