[clang] [openmp] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
https://github.com/doru1004 closed 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
[clang] [openmp] [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:
Will try to investigate it tomorrow, probably
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
[clang] [openmp] [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:
I can't find any "bug" in the existing code. It works as intended. The problem
is that it doesn't handle these types of situations and I don't see how else to
fix an ordering problem other than by re-ordering. If you have a different
solution 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
[clang] [openmp] [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:
Well I don't see anything other that's wrong other than the order and the order
comes from how the user wrote the code so I am not sure how else to fix it.
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
[clang] [openmp] [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:
And this is what should be fixed, no need to add a hack, that "hides" the bug
in the code, need to fix the bug.
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
[clang] [openmp] [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:
Implicit ordering isn't working in the case in the example above, please see
the code. The entries are in the wrong order in the runtime and the problem
starts here.
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
[clang] [openmp] [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:
But that what this code already does, it (implicitly) "reorders" the clauses by
extending the mapped region automatically. Explicit reordering should not be
required.
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
[clang] [openmp] [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:
at runtime, if things happen in the wrong order, the processing of the base
struct overwrites the pointer attachment for the array.
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
[clang] [openmp] [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:
It's a form of sorting it's more like a split between all section-containing
maps and the ones that don't.
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
[clang] [openmp] [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:
This sorting approach is not very good, it adds lots of the code which is not
required. It was supposed that this function automatically adjusts the
boundaries of the structs/classes, so sorting is not required.
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
[clang] [openmp] [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:
Ah yes, so I just moved all the maps containing sections at the end of the
clause list. I want those maps to happen last after all the structs and other
maps have happened.
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
[clang] [openmp] [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;
jhuber6 wrote:
It's called "SortedMapClauses" but I don't see any sorting, we just push back
into the vector.
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
[clang] [openmp] [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:
Are you asking what is the sorting criteria?
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
[clang] [openmp] [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:
I don't understand the question.
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
[clang] [openmp] [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;
jhuber6 wrote:
What implies that this is sorted?
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
[clang] [openmp] [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 2ea93a7b4841671dc12ee39a25a66c536d92d83f 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 | 23 +++
clang/test/OpenMP/map_struct_ordering.cpp | 172 ++
.../struct_mapping_with_pointers.cpp | 114
3 files changed, 309 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..b4b8794947687c0 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7731,10 +7731,31 @@ 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 non-section maps first to avoid overwriting pointer
+// attachment.
+for (const OMPMapClause *C : SortedMapClauses) {
MapKind Kind = Other;
if (llvm::is_contained(C->getMapTypeModifiers(),
OMPC_MAP_MODIFIER_present))
@@ -7751,6 +7772,7 @@ class MappableExprsHandler {
++EI;
}
}
+
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast(Cl);
if (!C)
@@ -7767,6 +7789,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:
[clang] [openmp] [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 d29229095203dccdee5ded18c0df0474e006ad53 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 | 27 ++-
clang/test/OpenMP/map_struct_ordering.cpp | 172 ++
.../struct_mapping_with_pointers.cpp | 114
3 files changed, 311 insertions(+), 2 deletions(-)
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..a39115300fa641e 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7731,10 +7731,31 @@ 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 non-section maps first to avoid overwriting pointer
+// attachment.
+for (const OMPMapClause *C : SortedMapClauses) {
MapKind Kind = Other;
if (llvm::is_contained(C->getMapTypeModifiers(),
OMPC_MAP_MODIFIER_present))
@@ -7746,11 +7767,12 @@ class MappableExprsHandler {
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);
+/*ReturnDevicePointer=*/false, C->isImplicit(),
+std::get<2>(L), E);
++EI;
}
}
+
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast(Cl);
if (!C)
@@ -7767,6 +7789,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]],
[clang] [openmp] [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;
+ if (*EI && !isa(*EI)) {
+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;
+}
+ }
+}
+
+// Process the maps with sections.
+for (const auto *Cl : Clauses) {
+ const auto *C = dyn_cast(Cl);
+ if (!C)
+continue;
+ MapKind Kind = Other;
+ if (llvm::is_contained(C->getMapTypeModifiers(),
+ OMPC_MAP_MODIFIER_present))
+Kind = Present;
+ else if (C->getMapType() == OMPC_MAP_alloc)
+Kind = Allocs;
+ const auto *EI = C->getVarRefs().begin();
+ if (*EI && isa(*EI)) {
+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;
+}
jdoerfert wrote:
This duplicates the loop nest, which is very unfortunate. Why not actually sort
the clause list? That will also make it easier to add/change things in the
future, e.g., we simply modify the comparator.
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
[clang] [openmp] [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 a16ffab67e8f8134fd943761da730c120bbae88d 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 | 44 -
clang/test/OpenMP/map_struct_ordering.cpp | 172 ++
.../struct_mapping_with_pointers.cpp | 114
3 files changed, 323 insertions(+), 7 deletions(-)
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..0079530f90f723d 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7731,6 +7731,8 @@ class MappableExprsHandler {
IsImplicit, Mapper, VarRef, ForDeviceAddr);
};
+// Iterate over all non-section maps first to avoid overwriting pointer
+// attachment.
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast(Cl);
if (!C)
@@ -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;
+ if (*EI && !isa(*EI)) {
+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;
+}
+ }
+}
+
+// Process the maps with sections.
+for (const auto *Cl : Clauses) {
+ const auto *C = dyn_cast(Cl);
+ if (!C)
+continue;
+ MapKind Kind = Other;
+ if (llvm::is_contained(C->getMapTypeModifiers(),
+ OMPC_MAP_MODIFIER_present))
+Kind = Present;
+ else if (C->getMapType() == OMPC_MAP_alloc)
+Kind = Allocs;
+ const auto *EI = C->getVarRefs().begin();
+ if (*EI && isa(*EI)) {
+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;
+}
}
}
+
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast(Cl);
if (!C)
@@ -7767,6 +7796,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],
[clang] [openmp] [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. No code gen test changes. Happy to add one no problem. 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
[clang] [openmp] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
https://github.com/jhuber6 commented: 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. 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
[clang] [openmp] [Clang][OpenMP] Fix ordering of processing of map clauses when mapping a struct. (PR #72410)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff cb1a6392ce541b74a700d9440f7c0a832ca77d2b 6f9450b5fa9ff47c35e7498b3a536a218655a9d6 -- openmp/libomptarget/test/offloading/struct_mapping_with_pointers.cpp clang/lib/CodeGen/CGOpenMPRuntime.cpp `` View the diff from clang-format here. ``diff diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp b/clang/lib/CodeGen/CGOpenMPRuntime.cpp index 50518c4615..0079530f90 100644 --- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp +++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp @@ -7749,8 +7749,8 @@ private: 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); + /*ReturnDevicePointer=*/false, C->isImplicit(), + std::get<2>(L), E); ++EI; } } @@ -7773,8 +7773,8 @@ private: 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); + /*ReturnDevicePointer=*/false, C->isImplicit(), + std::get<2>(L), E); ++EI; } } `` 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
[clang] [openmp] [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 ed9d50576cf167b4d9017e55333220d1601d088f 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 | 44 +--
.../struct_mapping_with_pointers.cpp | 114 ++
2 files changed, 151 insertions(+), 7 deletions(-)
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..0079530f90f723d 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7731,6 +7731,8 @@ class MappableExprsHandler {
IsImplicit, Mapper, VarRef, ForDeviceAddr);
};
+// Iterate over all non-section maps first to avoid overwriting pointer
+// attachment.
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast(Cl);
if (!C)
@@ -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;
+ if (*EI && !isa(*EI)) {
+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;
+}
+ }
+}
+
+// Process the maps with sections.
+for (const auto *Cl : Clauses) {
+ const auto *C = dyn_cast(Cl);
+ if (!C)
+continue;
+ MapKind Kind = Other;
+ if (llvm::is_contained(C->getMapTypeModifiers(),
+ OMPC_MAP_MODIFIER_present))
+Kind = Present;
+ else if (C->getMapType() == OMPC_MAP_alloc)
+Kind = Allocs;
+ const auto *EI = C->getVarRefs().begin();
+ if (*EI && isa(*EI)) {
+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;
+}
}
}
+
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast(Cl);
if (!C)
@@ -7767,6 +7796,7 @@ class MappableExprsHandler {
++EI;
}
}
+
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast(Cl);
if (!C)
diff --git
a/openmp/libomptarget/test/offloading/struct_mapping_with_pointers.cpp
b/openmp/libomptarget/test/offloading/struct_mapping_with_pointers.cpp
new file mode 100644
index 000..c7ce4bade8de9a2
--- /dev/null
+++ b/openmp/libomptarget/test/offloading/struct_mapping_with_pointers.cpp
@@ -0,0 +1,114 @@
+// clang-format off
+// RUN: %libomptarget-compilexx-generic && env LIBOMPTARGET_DEBUG=1
%libomptarget-run-generic 2>&1 | %fcheck-generic
+// clang-format on
+
+#include
+#include
+
+struct Descriptor {
+ int *datum;
+ long int x;
+ int *more_datum;
+ int xi;
+ int val_datum, val_more_datum;
+ long int arr[1][30];
+ int val_arr;
+};
+
+int main() {
+ Descriptor dat = Descriptor();
+ dat.datum = (int *)malloc(sizeof(int) * 10);
+ dat.more_datum = (int *)malloc(sizeof(int) * 20);
+ dat.xi = 3;
+ dat.arr[0][0] = 1;
+
+ dat.datum[7] = 7;
+ dat.more_datum[17] = 17;
+
+ /// The struct is mapped with type 0x0 when the pointer fields are mapped.
+ /// The struct is also map explicitely by the user. The second mapping by
+ /// the user must not overwrite the mapping set up for the pointer fields
+ /// when mapping the struct happens after the mapping of the pointers.
+
+ // clang-format off
+ // CHECK: Libomptarget --> Entry 0: Base=[[DAT_HST_PTR_BASE:0x.*]],
Begin=[[DAT_HST_PTR_BASE]], Size=288, Type=0x0, Name=unknown
+ // CHECK: Libomptarget --> Entry 1: Base=[[DAT_HST_PTR_BASE]],
Begin=[[DAT_HST_PTR_BASE]], Size=288, Type=0x10001, Name=unknown
+ // CHECK: Libomptarget --> Entry 2: Base=[[DAT_HST_PTR_BASE]],
Begin=[[DATUM_HST_PTR_BASE:0x.*]], Size=40, Type=0x10011, Name=unknown
+ // CHECK: Libomptarget --> Entry 3: Base=[[MORE_DATUM_HST_PTR_BASE:0x.*]],
Begin=[[MORE_DATUM_HST_PTR_BEGIN:0x.*]],
