This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Add parsing/sema/serialization for 'bind' clause
ClosedPublic

Authored by mikerice on Nov 3 2021, 5:32 PM.

Details

Summary

Add initial parsing, sema, and serialization support for 'bind', a new clause for the 'loop' directive.

Three bindings are allowed: teams, parallel, or thread.

Diff Detail

Event Timeline

mikerice created this revision.Nov 3 2021, 5:32 PM
mikerice requested review of this revision.Nov 3 2021, 5:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 3 2021, 5:32 PM
ABataev added inline comments.Nov 4 2021, 5:27 AM
clang/lib/Sema/SemaOpenMP.cpp
4706–4713

Do we allow something like this:

void foo() {
  #pragma omp loop // no bind
  ...
}
volid bar() {
  #pragma omp parallel
  foo();
}

?

clang/lib/Serialization/ASTReader.cpp
12962

Use C->setBindKind(Record.readEnum<OpenMPBindClauseKind>());

clang/lib/Serialization/ASTWriter.cpp
6736

Use Record.writeEnum(C->getBindKind());

mikerice added inline comments.Nov 4 2021, 8:35 AM
clang/lib/Sema/SemaOpenMP.cpp
4706–4713

I thought this is not allowed and a bind clause should be used. But there appears to be some room for allowing it. The spec also says:

If the bind clause is not present on the construct and the loop construct is closely nested inside a teams or parallel construct, the binding region is the corresponding teams or parallel region. If none of those conditions hold, the binding region is not defined.

If the binding region is not defined, then the binding thread set is the encountering thread.

Should I remove this check or leave it? It's not clear to me what the intentions are here.

mikerice updated this revision to Diff 384799.Nov 4 2021, 10:22 AM

Allow 'loop' without bind clause.
Use readEnum/writeEnum in serialization.

ABataev added inline comments.Nov 4 2021, 12:58 PM
llvm/include/llvm/Frontend/OpenMP/OMP.td
1746

Should this be VersionedClause<OMPC_bind, 51>?

mikerice marked 4 inline comments as done.Nov 4 2021, 1:56 PM
mikerice added inline comments.
llvm/include/llvm/Frontend/OpenMP/OMP.td
1746

This came in for 5.0 so I guess: VersionedClause<OMPC_Bind, 50>

mikerice updated this revision to Diff 384860.Nov 4 2021, 1:58 PM
mikerice marked an inline comment as done.

Added version to clause.

This revision is now accepted and ready to land.Nov 4 2021, 1:59 PM
This revision was landed with ongoing or failed builds.Nov 4 2021, 2:41 PM
This revision was automatically updated to reflect the committed changes.
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptNov 4 2021, 2:41 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript