[openmp] [clang] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
@@ -7731,10 +7731,30 @@ class MappableExprsHandler {
IsImplicit, Mapper, VarRef, ForDeviceAddr);
};
+// Sort all map clauses and make sure all the maps containing array
+// sections are processed last.
+llvm::SmallVector SortedMapClauses;
alexey-bataev wrote:
So, looks like we emit something like this as maptypes:
0x0
0x10011 - MEMBER_OF_1 | MAP_PTR_AND_OBJ | MAP_TO - array section
0x10001 - MEMBER_OF_1 | MAP_TO - whole struct
I think the whole struct info can be copied to the very first element instead
of the placeholder, if we see that the whole struct is mapped, and the
corresponding element can be removed out of the list. I think this can be done
in emitCombinedEntry function
https://github.com/llvm/llvm-project/pull/72410
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[openmp] [clang] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
@@ -7731,10 +7731,30 @@ class MappableExprsHandler {
IsImplicit, Mapper, VarRef, ForDeviceAddr);
};
+// Sort all map clauses and make sure all the maps containing array
+// sections are processed last.
+llvm::SmallVector SortedMapClauses;
doru1004 wrote:
@alexey-bataev I have looked at the code again and I really can't see another
solution to this problem. If you have a different fix in mind please let me
know.
https://github.com/llvm/llvm-project/pull/72410
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[openmp] [clang] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
@@ -7731,10 +7731,30 @@ class MappableExprsHandler {
IsImplicit, Mapper, VarRef, ForDeviceAddr);
};
+// Sort all map clauses and make sure all the maps containing array
+// sections are processed last.
+llvm::SmallVector SortedMapClauses;
alexey-bataev wrote:
Why, what's wrong with this?
https://github.com/llvm/llvm-project/pull/72410
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[openmp] [clang] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
@@ -7731,10 +7731,30 @@ class MappableExprsHandler {
IsImplicit, Mapper, VarRef, ForDeviceAddr);
};
+// Sort all map clauses and make sure all the maps containing array
+// sections are processed last.
+llvm::SmallVector SortedMapClauses;
doru1004 wrote:
@alexey-bataev I agree it's not ideal. the problem is related to the order in
which the clauses are processed. We cannot process the base struct after we
have processed an array section inside the struct.
https://github.com/llvm/llvm-project/pull/72410
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[openmp] [clang] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
https://github.com/doru1004 updated
https://github.com/llvm/llvm-project/pull/72410
>From 6712acd1175d1d6d55ce261651a543872a221c9a Mon Sep 17 00:00:00 2001
From: Doru Bercea
Date: Wed, 15 Nov 2023 11:07:09 -0500
Subject: [PATCH] Fix ordering when mapping a struct.
---
clang/lib/CodeGen/CGOpenMPRuntime.cpp | 22 +++
clang/test/OpenMP/map_struct_ordering.cpp | 172 ++
.../struct_mapping_with_pointers.cpp | 114
3 files changed, 308 insertions(+)
create mode 100644 clang/test/OpenMP/map_struct_ordering.cpp
create mode 100644
openmp/libomptarget/test/offloading/struct_mapping_with_pointers.cpp
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index d2be8141a3a4b31..84a6b36646897d7 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7731,10 +7731,30 @@ class MappableExprsHandler {
IsImplicit, Mapper, VarRef, ForDeviceAddr);
};
+// Sort all map clauses and make sure all the maps containing array
+// sections are processed last.
+llvm::SmallVector SortedMapClauses;
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast(Cl);
if (!C)
continue;
+ const auto *EI = C->getVarRefs().begin();
+ if (*EI && !isa(*EI)) {
+SortedMapClauses.emplace_back(C);
+ }
+}
+for (const auto *Cl : Clauses) {
+ const auto *C = dyn_cast(Cl);
+ if (!C)
+continue;
+ const auto *EI = C->getVarRefs().begin();
+ if (*EI && isa(*EI)) {
+SortedMapClauses.emplace_back(C);
+ }
+}
+
+// Iterate over all map clauses:
+for (const OMPMapClause *C : SortedMapClauses) {
MapKind Kind = Other;
if (llvm::is_contained(C->getMapTypeModifiers(),
OMPC_MAP_MODIFIER_present))
@@ -7751,6 +7771,7 @@ class MappableExprsHandler {
++EI;
}
}
+
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast(Cl);
if (!C)
@@ -7767,6 +7788,7 @@ class MappableExprsHandler {
++EI;
}
}
+
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast(Cl);
if (!C)
diff --git a/clang/test/OpenMP/map_struct_ordering.cpp
b/clang/test/OpenMP/map_struct_ordering.cpp
new file mode 100644
index 000..035b39b5b12ab4a
--- /dev/null
+++ b/clang/test/OpenMP/map_struct_ordering.cpp
@@ -0,0 +1,172 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
UTC_ARGS: --include-generated-funcs --replace-value-regex
"__omp_offloading_[0-9a-z]+_[0-9a-z]+" --prefix-filecheck-ir-name _ --version 4
+
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -std=c++11 -triple
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu
-emit-llvm %s -o - -Wno-openmp-mapping | FileCheck %s --check-prefix=CHECK
+
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+struct Descriptor {
+ int *datum;
+ long int x;
+ int xi;
+ long int arr[1][30];
+};
+
+int map_struct() {
+ Descriptor dat = Descriptor();
+ dat.xi = 3;
+ dat.arr[0][0] = 1;
+
+ #pragma omp target enter data map(to: dat.datum[:10]) map(to: dat)
+
+ #pragma omp target
+ {
+dat.xi = 4;
+dat.datum[dat.arr[0][0]] = dat.xi;
+ }
+
+ #pragma omp target exit data map(from: dat)
+
+ return dat.xi;
+}
+
+#endif
+// CHECK-LABEL: define dso_local noundef signext i32 @_Z10map_structv(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:[[DAT:%.*]] = alloca [[STRUCT_DESCRIPTOR:%.*]], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_BASEPTRS:%.*]] = alloca [3 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_PTRS:%.*]] = alloca [3 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_MAPPERS:%.*]] = alloca [3 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_SIZES:%.*]] = alloca [3 x i64], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_BASEPTRS4:%.*]] = alloca [1 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_PTRS5:%.*]] = alloca [1 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_MAPPERS6:%.*]] = alloca [1 x ptr], align 8
+// CHECK-NEXT:[[KERNEL_ARGS:%.*]] = alloca
[[STRUCT___TGT_KERNEL_ARGUMENTS:%.*]], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_BASEPTRS7:%.*]] = alloca [1 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_PTRS8:%.*]] = alloca [1 x ptr], align 8
+// CHECK-NEXT:[[DOTOFFLOAD_MAPPERS9:%.*]] = alloca [1 x ptr], align 8
+// CHECK-NEXT:call void @llvm.memset.p0.i64(ptr align 8 [[DAT]], i8 0, i64
264, i1 false)
+// CHECK-NEXT:[[XI:%.*]] = getelementptr inbounds [[STRUCT_DESCRIPTOR]],
ptr [[DAT]], i32 0, i32 2
+// CHECK-NEXT:store i32 3, ptr [[XI]], align 8
+// CHECK-NEXT:[[ARR:%.*]] = getelementptr inbounds [[STRUCT_DESCRIPTOR]],
ptr [[DAT]], i32 0, i32 3
+// CHECK-NEXT:[[ARRAYIDX:%.*]] = getelementptr inbounds [1 x [30 x i64]],
ptr [[ARR]], i64 0, i64 0
+// CHECK-NEXT:[[ARRAYIDX1:%.*]] = getelementptr inbounds [30 x i64], ptr
[openmp] [clang] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
@@ -7742,15 +7744,42 @@ class MappableExprsHandler {
else if (C->getMapType() == OMPC_MAP_alloc)
Kind = Allocs;
const auto *EI = C->getVarRefs().begin();
- for (const auto L : C->component_lists()) {
-const Expr *E = (C->getMapLoc().isValid()) ? *EI : nullptr;
-InfoGen(std::get<0>(L), Kind, std::get<1>(L), C->getMapType(),
-C->getMapTypeModifiers(), std::nullopt,
-/*ReturnDevicePointer=*/false, C->isImplicit(), std::get<2>(L),
-E);
-++EI;
alexey-bataev wrote:
That's strange that array sections are not handled correctly here, need to
check why componen_list() does not return it correctly. Or just InfoGen does
not process it correctly.
https://github.com/llvm/llvm-project/pull/72410
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[openmp] [clang] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
doru1004 wrote: > This being in clang instead seems like a good change. Are there no CodeGen > tests changed? We should add one if so. Probably just take your > `libomptarget` test and run `update_cc_test_checks` on it with the arguments > found in other test files. Just added the test. https://github.com/llvm/llvm-project/pull/72410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[openmp] [clang] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
https://github.com/doru1004 edited https://github.com/llvm/llvm-project/pull/72410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
