llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-codegen
Author: Gheorghe-Teodor Bercea (doru1004)
<details>
<summary>Changes</summary>
Mapping a struct, if done in the wrong order, can overwrite the pointer
attachment details. This fixes this problem.
Original failing example:
```
#include <stdio.h>
#include <stdlib.h>
struct Descriptor {
int *datum;
long int x;
int xi;
long int arr[1][30];
};
int main() {
Descriptor dat = Descriptor();
dat.datum = (int *)malloc(sizeof(int)*10);
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 0;
}
```
Previous attempt at fixing this: https://github.com/llvm/llvm-project/pull/70821
---
Full diff: https://github.com/llvm/llvm-project/pull/72410.diff
2 Files Affected:
- (modified) clang/lib/CodeGen/CGOpenMPRuntime.cpp (+37-7)
- (added) openmp/libomptarget/test/offloading/struct_mapping_with_pointers.cpp
(+114)
``````````diff
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index d2be8141a3a4b31..50518c46152bbaf 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<OMPMapClause>(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<OMPArraySectionExpr>(*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<OMPMapClause>(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<OMPArraySectionExpr>(*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<OMPToClause>(Cl);
if (!C)
@@ -7767,6 +7796,7 @@ class MappableExprsHandler {
++EI;
}
}
+
for (const auto *Cl : Clauses) {
const auto *C = dyn_cast<OMPFromClause>(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 000000000000000..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 <stdio.h>
+#include <stdlib.h>
+
+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=0x1000000000001, Name=unknown
+ // CHECK: Libomptarget --> Entry 2: Base=[[DAT_HST_PTR_BASE]],
Begin=[[DATUM_HST_PTR_BASE:0x.*]], Size=40, Type=0x1000000000011, Name=unknown
+ // CHECK: Libomptarget --> Entry 3: Base=[[MORE_DATUM_HST_PTR_BASE:0x.*]],
Begin=[[MORE_DATUM_HST_PTR_BEGIN:0x.*]], Size=80, Type=0x1000000000011,
Name=unknown
+ // clang-format on
+
+ /// The struct will be mapped in the same order as the above entries.
+
+ /// First argument is the struct itself and it will be mapped once.
+
+ // clang-format off
+ // CHECK: Libomptarget --> Looking up
mapping(HstPtrBegin=[[DAT_HST_PTR_BASE]], Size=288)...
+ // CHECK: PluginInterface --> MemoryManagerTy::allocate: size 288 with host
pointer [[DAT_HST_PTR_BASE]].
+ // CHECK: Libomptarget --> Creating new map entry with
HstPtrBase=[[DAT_HST_PTR_BASE]], HstPtrBegin=[[DAT_HST_PTR_BASE]],
TgtAllocBegin=[[DAT_DEVICE_PTR_BASE:0x.*]],
TgtPtrBegin=[[DAT_DEVICE_PTR_BASE]], Size=288, DynRefCount=1, HoldRefCount=0,
Name=unknown
+ // CHECK: Libomptarget --> Moving 288 bytes (hst:[[DAT_HST_PTR_BASE]]) ->
(tgt:[[DAT_DEVICE_PTR_BASE]])
+ // clang-format on
+
+ /// Second argument is dat.datum:
+ // clang-format off
+ // CHECK: Libomptarget --> Looking up
mapping(HstPtrBegin=[[DATUM_HST_PTR_BASE]], Size=40)...
+ // CHECK: PluginInterface --> MemoryManagerTy::allocate: size 40 with host
pointer [[DATUM_HST_PTR_BASE]].
+ // CHECK: Libomptarget --> Creating new map entry with
HstPtrBase=[[DATUM_HST_PTR_BASE]], HstPtrBegin=[[DATUM_HST_PTR_BASE]],
TgtAllocBegin=[[DATUM_DEVICE_PTR_BASE:0x.*]],
TgtPtrBegin=[[DATUM_DEVICE_PTR_BASE]], Size=40, DynRefCount=1, HoldRefCount=0,
Name=unknown
+ // CHECK: Libomptarget --> Moving 40 bytes (hst:[[DATUM_HST_PTR_BASE]]) ->
(tgt:[[DATUM_DEVICE_PTR_BASE]])
+ // clang-format on
+
+ /// Third argument is dat.more_datum:
+ // clang-format off
+ // CHECK: Libomptarget --> Looking up
mapping(HstPtrBegin=[[MORE_DATUM_HST_PTR_BEGIN]], Size=80)...
+ // CHECK: PluginInterface --> MemoryManagerTy::allocate: size 80 with host
pointer [[MORE_DATUM_HST_PTR_BEGIN]].
+ // CHECK: Libomptarget --> Creating new map entry with
HstPtrBase=[[MORE_DATUM_HST_PTR_BEGIN]],
HstPtrBegin=[[MORE_DATUM_HST_PTR_BEGIN]],
TgtAllocBegin=[[MORE_DATUM_DEVICE_PTR_BEGIN:0x.*]],
TgtPtrBegin=[[MORE_DATUM_DEVICE_PTR_BEGIN]], Size=80, DynRefCount=1,
HoldRefCount=0, Name=unknown
+ // CHECK: Libomptarget --> Moving 80 bytes
(hst:[[MORE_DATUM_HST_PTR_BEGIN]]) -> (tgt:[[MORE_DATUM_DEVICE_PTR_BEGIN]])
+ // clang-format on
+
+#pragma omp target enter data map(to : dat.datum[ : 10])
\
+ map(to : dat.more_datum[ : 20]) map(to : dat)
+
+ /// Checks induced by having a target region:
+ // clang-format off
+ // CHECK: Libomptarget --> Entry 0: Base=[[DAT_HST_PTR_BASE]],
Begin=[[DAT_HST_PTR_BASE]], Size=288, Type=0x223, Name=unknown
+ // CHECK: Libomptarget --> Mapping exists (implicit) with
HstPtrBegin=[[DAT_HST_PTR_BASE]], TgtPtrBegin=[[DAT_DEVICE_PTR_BASE]],
Size=288, DynRefCount=2 (incremented), HoldRefCount=0, Name=unknown
+ // CHECK: Libomptarget --> Obtained target argument [[DAT_DEVICE_PTR_BASE]]
from host pointer [[DAT_HST_PTR_BASE]]
+ // clang-format on
+
+#pragma omp target
+ {
+ dat.xi = 4;
+ dat.datum[7]++;
+ dat.more_datum[17]++;
+ dat.val_datum = dat.datum[7];
+ dat.val_more_datum = dat.more_datum[17];
+ dat.datum[dat.arr[0][0]] = dat.xi;
+ dat.val_arr = dat.datum[dat.arr[0][0]];
+ }
+
+ /// Post-target region checks:
+ // clang-format off
+ // CHECK: Libomptarget --> Mapping exists with
HstPtrBegin=[[DAT_HST_PTR_BASE]], TgtPtrBegin=[[DAT_DEVICE_PTR_BASE]],
Size=288, DynRefCount=1 (decremented), HoldRefCount=0
+ // clang-format on
+
+#pragma omp target exit data map(from : dat)
+
+ /// Target data end checks:
+ // clang-format off
+ // CHECK: Libomptarget --> Mapping exists with
HstPtrBegin=[[DAT_HST_PTR_BASE]], TgtPtrBegin=[[DAT_DEVICE_PTR_BASE]],
Size=288, DynRefCount=0 (decremented, delayed deletion), HoldRefCount=0
+ // CHECK: Libomptarget --> Moving 288 bytes (tgt:[[DAT_DEVICE_PTR_BASE]]) ->
(hst:[[DAT_HST_PTR_BASE]])
+ // clang-format on
+
+ // CHECK: dat.xi = 4
+ // CHECK: dat.val_datum = 8
+ // CHECK: dat.val_more_datum = 18
+ // CHECK: dat.datum[dat.arr[0][0]] = 0
+ // CHECK: dat.val_arr = 4
+
+ printf("dat.xi = %d\n", dat.xi);
+ printf("dat.val_datum = %d\n", dat.val_datum);
+ printf("dat.val_more_datum = %d\n", dat.val_more_datum);
+ printf("dat.datum[dat.arr[0][0]] = %d\n", dat.datum[dat.arr[0][0]]);
+ printf("dat.val_arr = %d\n", dat.val_arr);
+
+ return 0;
+}
``````````
</details>
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
