Support for Code Generation of "#pragma loop bind" clause.
- bind(parallel)
- bind(teams)
- bind(thread)
Paths
| Differential D144634
[Clang][OpenMP] Support for Code Generation of loop bind clause ClosedPublic Authored by koops on Feb 23 2023, 3:14 AM.
Details Summary Support for Code Generation of "#pragma loop bind" clause.
Diff Detail Event TimelineComment Actions bind(thread) is not working at present. I have uploaded this patch to obtain feedback mainly on this. Comment Actions We need tests.
carlo.bertolli added inline comments.
Comment Actions
Comment Actions Removing changes from :
These were useful when the code was in CodeGen to handle the bind clause.
Comment Actions
Comment Actions
But why need to drop it? It makes processing more complex. The bind clause itself should not be a problem, no?
Comment Actions
However, generic_loop_messages.cpp & generic_loop_ast_print.cpp are present and provide a good coverage. Comment Actions
I rather doubt that these tests provide good coverage, since you're changing the directive kind here on the fly. This is a very new functionality, which was not tested before.
Comment Actions
In https://reviews.llvm.org/D145823 : Add codegen for combined 'loop' directives, I see the following code emitCommonOMPParallelDirective(*this, S, OMPD_for, CodeGen, emitEmptyBoundParameters); .... void CodeGenFunction::EmitOMPTeamsGenericLoopDirective() { CGF.CGM.getOpenMPRuntime().emitInlinedDirective(CGF, OMPD_distribute, CodeGenDistribute); .... Comment Actions
Comment Actions
Comment Actions
Comment Actions Addressing Alexey's comments. Comment Actions
Comment Actions Moving the variable MappedDirective into structure SharingMapTy. Enough comments have been put in place to explain.
Comment Actions
Comment Actions
Comment Actions
i) Instead of calling setMappedDirective() after the the creation of the Directive, made the MappedDirective a parameter of the Create() method.
Comment Actions
This revision is now accepted and ready to land.Aug 4 2023, 12:13 PM Comment Actions In clang/test/OpenMP/loop_bind_enclosed.cpp Generalizing the CHECK pattern for aarch64-linux, s390x-linux and ppc64le-linux. Comment Actions Can someone please check for MacOS? Yesterday when this support was committed, the CHECK statements in tests loop_bind_codegen.cpp and loop_bind_enclosed.cpp had failed on Mac and I received a comment as follows: When relanding, please remember to put a link to the review in the commit message. I have made the CHECK statements generic enough to match those on Mac with CHECK for the functions that the test were expected. Comment Actions
I patched this in and verified that check-clang now passes on macOS. Thanks!
To do this, run git commit --amend and put Differential Revision: https://reviews.llvm.org/D144634 at the bottom of the commit message. git log has many, many examples of this :) Closed by commit rG8ab62da18d47: [Clang][OpenMP] Support for Code Generation of loop bind clause (authored by cchen). · Explain WhyAug 9 2023, 12:26 PM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 544428 clang/include/clang/AST/StmtOpenMP.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/TreeTransform.h
clang/test/OpenMP/generic_loop_ast_print.cpp
clang/test/OpenMP/generic_loop_codegen.cpp
clang/test/OpenMP/loop_bind_codegen.cpp
clang/test/OpenMP/loop_bind_enclosed.cpp
clang/test/OpenMP/loop_bind_messages.cpp
clang/test/OpenMP/nested_loop_codegen.cpp
|
OpenMPDirectiveKind PrevMappedDirective = llvm::omp::OMPD_unknown;