[OpenMP] Clang Codegen Interop : Accept multiple init #82604
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 4 commits
February 20, 2024 11:29
Changes to be committed: modified: clang/lib/CodeGen/CGStmtOpenMP.cpp
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: None (SunilKuravinakop) ChangesModifying clang/lib/CodeGen/CGStmtOpenMP.cpp to accept multiple Full diff: https://github.com/llvm/llvm-project/pull/82604.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 8fd74697de3c0f..c512b0bd851f7a 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -7023,19 +7023,25 @@ void CodeGenFunction::EmitOMPInteropDirective(const OMPInteropDirective &S) {
S.getSingleClause<OMPUseClause>())) &&
"OMPNowaitClause clause is used separately in OMPInteropDirective.");
- if (const auto *C = S.getSingleClause<OMPInitClause>()) {
- llvm::Value *InteropvarPtr =
- EmitLValue(C->getInteropVar()).getPointer(*this);
- llvm::omp::OMPInteropType InteropType = llvm::omp::OMPInteropType::Unknown;
- if (C->getIsTarget()) {
- InteropType = llvm::omp::OMPInteropType::Target;
- } else {
- assert(C->getIsTargetSync() && "Expected interop-type target/targetsync");
- InteropType = llvm::omp::OMPInteropType::TargetSync;
+ auto It = S.getClausesOfKind<OMPInitClause>();
+ if (!It.empty()) {
+ // Look at the multiple init clauses
+ for (auto C : It) {
+ llvm::Value *InteropvarPtr =
+ EmitLValue(C->getInteropVar()).getPointer(*this);
+ llvm::omp::OMPInteropType InteropType =
+ llvm::omp::OMPInteropType::Unknown;
+ if (C->getIsTarget()) {
+ InteropType = llvm::omp::OMPInteropType::Target;
+ } else {
+ assert(C->getIsTargetSync() &&
+ "Expected interop-type target/targetsync");
+ InteropType = llvm::omp::OMPInteropType::TargetSync;
+ }
+ OMPBuilder.createOMPInteropInit(Builder, InteropvarPtr, InteropType,
+ Device, NumDependences, DependenceList,
+ Data.HasNowaitClause);
}
- OMPBuilder.createOMPInteropInit(Builder, InteropvarPtr, InteropType, Device,
- NumDependences, DependenceList,
- Data.HasNowaitClause);
} else if (const auto *C = S.getSingleClause<OMPDestroyClause>()) {
llvm::Value *InteropvarPtr =
EmitLValue(C->getInteropVar()).getPointer(*this);
|
|
The test is required |
added 3 commits
February 24, 2024 00:41
provided twice. This check is being done only for AMD GPU. Changes to be committed: new file: clang/test/OpenMP/interop_codegen.cpp
-fopenmp-targets=powerpc64le-ibm-linux-gnu. Changes to be committed: modified: clang/test/OpenMP/interop_codegen.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
Outdated
| auto It = S.getClausesOfKind<OMPInitClause>(); | ||
| if (!It.empty()) { | ||
| // Look at the multiple init clauses | ||
| for (auto C : It) { |
There was a problem hiding this comment.
For the Iterator variable It auto It is preferable rather than
llvm::iterator_range<clang::OMPExecutableDirective::specific_clause_iterator<clang::OMPInitClause> > It
There was a problem hiding this comment.
C is just OMPClause * here, not an iterator
There was a problem hiding this comment.
I asking about auto for C, not for It.
added 2 commits
February 24, 2024 12:46
…*C : It)". Changes to be committed: modified: clang/lib/CodeGen/CGStmtOpenMP.cpp
Changes to be committed: modified: clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
Outdated
| auto It = S.getClausesOfKind<OMPInitClause>(); | ||
| if (!It.empty()) { | ||
| // Look at the multiple init clauses | ||
| for (const class OMPInitClause *C : It) { |
There was a problem hiding this comment.
Drop 'class', it won't compile
Changes to be committed: modified: clang/lib/CodeGen/CGStmtOpenMP.cpp modified: clang/test/OpenMP/interop_codegen.cpp
|
The recent commit also takes care of the feedback provided by Alexey. |
clang/lib/CodeGen/CGStmtOpenMP.cpp
Outdated
Comment on lines
7046
to
7066
| auto ItOMPDestroyClause = S.getClausesOfKind<OMPDestroyClause>(); | ||
| if (!ItOMPDestroyClause.empty()) { | ||
| // Look at the multiple destroy clauses | ||
| for (const OMPDestroyClause *C : ItOMPDestroyClause) { | ||
| llvm::Value *InteropvarPtr = | ||
| EmitLValue(C->getInteropVar()).getPointer(*this); | ||
| OMPBuilder.createOMPInteropDestroy(Builder, InteropvarPtr, Device, | ||
| NumDependences, DependenceList, | ||
| Data.HasNowaitClause); | ||
| } | ||
| } | ||
| auto ItOMPUseClause = S.getClausesOfKind<OMPUseClause>(); | ||
| if (!ItOMPUseClause.empty()) { | ||
| // Look at the multiple use clauses | ||
| for (const OMPUseClause *C : ItOMPUseClause) { | ||
| llvm::Value *InteropvarPtr = | ||
| EmitLValue(C->getInteropVar()).getPointer(*this); | ||
| OMPBuilder.createOMPInteropUse(Builder, InteropvarPtr, Device, | ||
| NumDependences, DependenceList, | ||
| Data.HasNowaitClause); | ||
| } |
There was a problem hiding this comment.
Split it to a separate patch
merge. Changes to be committed: modified: clang/lib/CodeGen/CGStmtOpenMP.cpp modified: clang/test/OpenMP/interop_codegen.cpp
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Modifying clang/lib/CodeGen/CGStmtOpenMP.cpp to accept multiple
initclauses withinteropdirective.