This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Initial parsing/sema for the 'omp loop' construct
ClosedPublic

Authored by mikerice on Oct 25 2021, 4:28 PM.

Details

Summary

Adds basic parsing/sema/serialization support for the #pragma omp loop
directive.

Diff Detail

Event Timeline

mikerice created this revision.Oct 25 2021, 4:28 PM
mikerice requested review of this revision.Oct 25 2021, 4:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 25 2021, 4:28 PM
ABataev added inline comments.Oct 26 2021, 6:13 AM
clang/include/clang/AST/StmtOpenMP.h
5490

final

clang/lib/Basic/OpenMPKinds.cpp
683

Why it cannot be just OMPD_unknown here just like for omp for directive? Should it be outlined as a function?

clang/lib/CodeGen/CGStmt.cpp
397

Better just to emit it as an inlined directive.

martong removed a subscriber: martong.Oct 26 2021, 9:42 AM

Thanks for the review.

clang/lib/Basic/OpenMPKinds.cpp
683

I didn't write any codegen for this but my impression was that 'loop' can be treated like a 'parallel for' depending on the binding/situation and need outlining. That's why I did this. If that isn't the right assumption I can change it.

clang/lib/CodeGen/CGStmt.cpp
397

Not sure exactly what you mean by inlined, but if you have something trivial for me to add. I wasn't planning to implement any codegen in this patch.

ABataev added inline comments.Oct 27 2021, 5:58 AM
clang/lib/Basic/OpenMPKinds.cpp
683

Hmm, not sure if this is correct. Could you clarify a bit on the expected codegen for different kinds of bindings? Some examples in pseudocode, maybe?

clang/lib/CodeGen/CGStmt.cpp
397

I just meant that if we can emit the associated statement as is, better to do it.

auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &Action) {
  CGF.EmitStmt(S.getAssociatedStmt());
};
OMPLexicalScope Scope(*this, S, OMPD_unknown);
CGM.getOpenMPRuntime().emitInlinedDirective(*this, OMPD_loop, CodeGen);

or something like this.

mikerice updated this revision to Diff 382786.Oct 27 2021, 2:11 PM

Added 'final' to OMPGenericLoopDirective.
Use OMPD_unknown for capture region at least temporarily.
Inline the underlying statement instead of assert in codegen.
Fix nesting check and added related test case.

ABataev added inline comments.Oct 27 2021, 2:30 PM
clang/lib/Sema/SemaOpenMP.cpp
3911

Here it should also be handled just like OMPD_simd or OMPD_for with TODO/FIXME.

mikerice updated this revision to Diff 382808.Oct 27 2021, 3:03 PM

Treat OMPD_loop like OMPD_simd/OMPD_for in ActOnOpenMPRegionStart.

This revision is now accepted and ready to land.Oct 27 2021, 3:06 PM
This revision was landed with ongoing or failed builds.Oct 28 2021, 8:27 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 28 2021, 8:27 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript