[clang] [llvm] [Clang][OpenMP] Non-contiguous strided update (PR #144635)
https://github.com/amitamd7 created
https://github.com/llvm/llvm-project/pull/144635
This patch handles the strided update in the `#pragma omp target update
from(data[a:b:c])` directive where 'c' represents the strided access leading to
non-contiguous update in the `data` array when the offloaded execution returns
the control back to host from device using the `from` clause.
Issue: Clang CodeGen where info is generated for the particular `MapType` (to,
from, etc), it was failing to detect the strided access. Because of this, the
`MapType` bits were incorrect when passed to runtime. This led to incorrect
execution (contiguous) in the libomptarget runtime code.
Added a minimal testcase that verifies the working of the patch.
>From 6846880a245a199b31f5cbbc0e9781460dc185ba Mon Sep 17 00:00:00 2001
From: amtiwari
Date: Mon, 16 Jun 2025 01:07:01 -0400
Subject: [PATCH 1/2] strided_update_offloading
---
clang/lib/CodeGen/CGOpenMPRuntime.cpp | 35 ++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 4173355491fd4..81a2dd0fae5c9 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7384,7 +7384,40 @@ class MappableExprsHandler {
// dimension.
uint64_t DimSize = 1;
-bool IsNonContiguous = CombinedInfo.NonContigInfo.IsNonContiguous;
+// Detects non-contiguous updates due to strided accesses.
+// Sets the 'IsNonContiguous' flag so that the 'MapType' bits are set
+// correctly when generating information to be passed to the runtime. The
+// flag is set to true if any array section has a stride not equal to 1, or
+// if the stride is not a constant expression (conservatively assumed
+// non-contiguous).
+bool IsNonContiguous = false;
+for (const auto &Component : Components) {
+ const auto *OASE =
+ dyn_cast(Component.getAssociatedExpression());
+ if (OASE) {
+const Expr *StrideExpr = OASE->getStride();
+if (StrideExpr) {
+ // Check if the stride is a constant integer expression
+ if (StrideExpr->isIntegerConstantExpr(CGF.getContext())) {
+if (auto Constant =
+StrideExpr->getIntegerConstantExpr(CGF.getContext())) {
+ int64_t StrideVal = Constant->getExtValue();
+ if (StrideVal != 1) {
+// Set flag if stride is not 1 (i.e., non-contiguous update)
+IsNonContiguous = true;
+break;
+ }
+}
+ } else {
+// If stride is not a constant, conservatively treat as
+// non-contiguous
+IsNonContiguous = true;
+break;
+ }
+}
+ }
+}
+
bool IsPrevMemberReference = false;
bool IsPartialMapped =
>From 31b83e221c03bee590182ed6c692dd8206a6e833 Mon Sep 17 00:00:00 2001
From: amtiwari
Date: Tue, 17 Jun 2025 04:04:03 -0400
Subject: [PATCH 2/2] bug-tested
---
offload/test/offloading/strided_update.c | 51
1 file changed, 51 insertions(+)
create mode 100644 offload/test/offloading/strided_update.c
diff --git a/offload/test/offloading/strided_update.c
b/offload/test/offloading/strided_update.c
new file mode 100644
index 0..fc47216fb5684
--- /dev/null
+++ b/offload/test/offloading/strided_update.c
@@ -0,0 +1,51 @@
+// Checks that "update from" clause in OpenMP is supported when the elements
are updated in a non-contiguous manner.
+// RUN: %libomptarget-compile-run-and-check-generic
+#include
+#include
+
+int main() {
+ int len = 8;
+ double data[len];
+ #pragma omp target map(tofrom: len, data[0:len])
+ {
+for (int i = 0; i < len; i++) {
+ data[i] = i;
+}
+ }
+ // initial values
+ printf("original host array values:\n");
+ for (int i = 0; i < len; i++)
+printf("%f\n", data[i]);
+ printf("\n");
+
+ #pragma omp target data map(to: len, data[0:len])
+ {
+#pragma omp target
+for (int i = 0; i < len; i++) {
+ data[i] += i ;
+}
+
+#pragma omp target update from(data[0:8:2])
+ }
+ // from results
+ // CHECK: 0.00
+ // CHECK: 1.00
+ // CHECK: 4.00
+ // CHECK: 3.00
+ // CHECK: 8.00
+ // CHECK: 5.00
+ // CHECK: 12.00
+ // CHECK: 7.00
+ // CHECK-NOT: 2.00
+ // CHECK-NOT: 6.00
+ // CHECK-NOT: 10.00
+ // CHECK-NOT: 14.00
+
+ printf("from target array results:\n");
+ for (int i = 0; i < len; i++)
+printf("%f\n", data[i]);
+ printf("\n");
+
+ return 0;
+}
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][OpenMP] Non-contiguous strided update (PR #144635)
https://github.com/amitamd7 converted_to_draft https://github.com/llvm/llvm-project/pull/144635 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][OpenMP] Non-contiguous strided update (PR #144635)
@@ -7384,7 +7384,40 @@ class MappableExprsHandler {
// dimension.
uint64_t DimSize = 1;
-bool IsNonContiguous = CombinedInfo.NonContigInfo.IsNonContiguous;
+// Detects non-contiguous updates due to strided accesses.
+// Sets the 'IsNonContiguous' flag so that the 'MapType' bits are set
+// correctly when generating information to be passed to the runtime. The
+// flag is set to true if any array section has a stride not equal to 1, or
+// if the stride is not a constant expression (conservatively assumed
+// non-contiguous).
+bool IsNonContiguous = false;
+for (const auto &Component : Components) {
+ const auto *OASE =
+ dyn_cast(Component.getAssociatedExpression());
+ if (OASE) {
+const Expr *StrideExpr = OASE->getStride();
+if (StrideExpr) {
+ // Check if the stride is a constant integer expression
+ if (StrideExpr->isIntegerConstantExpr(CGF.getContext())) {
amitamd7 wrote:
Yes, because `stride` can be a variable/complex expression that is determined
only at runtime, so conservatively treating it as non-contiguous.
https://github.com/llvm/llvm-project/pull/144635
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][OpenMP] Non-contiguous strided update (PR #144635)
@@ -7384,7 +7384,40 @@ class MappableExprsHandler {
// dimension.
uint64_t DimSize = 1;
-bool IsNonContiguous = CombinedInfo.NonContigInfo.IsNonContiguous;
+// Detects non-contiguous updates due to strided accesses.
+// Sets the 'IsNonContiguous' flag so that the 'MapType' bits are set
+// correctly when generating information to be passed to the runtime. The
+// flag is set to true if any array section has a stride not equal to 1, or
+// if the stride is not a constant expression (conservatively assumed
+// non-contiguous).
+bool IsNonContiguous = false;
+for (const auto &Component : Components) {
+ const auto *OASE =
+ dyn_cast(Component.getAssociatedExpression());
+ if (OASE) {
+const Expr *StrideExpr = OASE->getStride();
+if (StrideExpr) {
+ // Check if the stride is a constant integer expression
+ if (StrideExpr->isIntegerConstantExpr(CGF.getContext())) {
amitamd7 wrote:
Okay, got it. No, it does not implicitly check if it is an integer.
https://github.com/llvm/llvm-project/pull/144635
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][OpenMP] Non-contiguous strided update (PR #144635)
https://github.com/amitamd7 updated
https://github.com/llvm/llvm-project/pull/144635
>From 1383c0e58feff9aabbffab23dc705c497baa0f2d Mon Sep 17 00:00:00 2001
From: amtiwari
Date: Mon, 16 Jun 2025 01:07:01 -0400
Subject: [PATCH] strided_update_offloading with lit-tests added
---
clang/lib/CodeGen/CGOpenMPRuntime.cpp | 189 ++
.../test/offloading/strided_multiple_update.c | 61 ++
.../test/offloading/strided_partial_update.c | 63 ++
offload/test/offloading/strided_update.c | 54 +
4 files changed, 282 insertions(+), 85 deletions(-)
create mode 100644 offload/test/offloading/strided_multiple_update.c
create mode 100644 offload/test/offloading/strided_partial_update.c
create mode 100644 offload/test/offloading/strided_update.c
diff --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 8ccc37ef98a74..785eb5f6a869d 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -490,11 +490,11 @@ enum OpenMPLocationFlags : unsigned {
/// member */
///kmp_int32 reserved_2; /**< not really used in Fortran any more;
/// see above */
-///#if USE_ITT_BUILD
+/// #if USE_ITT_BUILD
////* but currently used for storing
///region-specific ITT */
////* contextual information. */
-///#endif /* USE_ITT_BUILD */
+/// #endif /* USE_ITT_BUILD */
///kmp_int32 reserved_3; /**< source[4] in Fortran, do not use for
/// C++ */
///char const *psource;/**< String describing the source location.
@@ -714,16 +714,16 @@ static void EmitOMPAggregateInit(CodeGenFunction &CGF,
Address DestAddr,
if (DRD) {
// Shift the address forward by one element.
-llvm::Value *SrcElementNext = CGF.Builder.CreateConstGEP1_32(
-SrcAddr.getElementType(), SrcElementPHI, /*Idx0=*/1,
-"omp.arraycpy.dest.element");
+llvm::Value *SrcElementNext =
+CGF.Builder.CreateConstGEP1_32(SrcAddr.getElementType(), SrcElementPHI,
+ /*Idx0=*/1,
"omp.arraycpy.dest.element");
SrcElementPHI->addIncoming(SrcElementNext, CGF.Builder.GetInsertBlock());
}
// Shift the address forward by one element.
- llvm::Value *DestElementNext = CGF.Builder.CreateConstGEP1_32(
- DestAddr.getElementType(), DestElementPHI, /*Idx0=*/1,
- "omp.arraycpy.dest.element");
+ llvm::Value *DestElementNext =
+ CGF.Builder.CreateConstGEP1_32(DestAddr.getElementType(), DestElementPHI,
+ /*Idx0=*/1, "omp.arraycpy.dest.element");
// Check whether we've reached the end.
llvm::Value *Done =
CGF.Builder.CreateICmpEQ(DestElementNext, DestEnd, "omp.arraycpy.done");
@@ -973,8 +973,8 @@ Address
ReductionCodeGen::adjustPrivateAddress(CodeGenFunction &CGF, unsigned N,
llvm::Value *PrivatePointer =
CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
PrivateAddr.emitRawPointer(CGF), SharedAddr.getType());
-llvm::Value *Ptr = CGF.Builder.CreateGEP(
-SharedAddr.getElementType(), PrivatePointer, Adjustment);
+llvm::Value *Ptr = CGF.Builder.CreateGEP(SharedAddr.getElementType(),
+ PrivatePointer, Adjustment);
return castToBase(CGF, OrigVD->getType(),
SharedAddresses[N].first.getType(),
OriginalBaseLValue.getAddress(), Ptr);
@@ -1599,12 +1599,11 @@ Address
CGOpenMPRuntime::getAddrOfThreadPrivate(CodeGenFunction &CGF,
CGF.Builder.CreatePointerCast(VDAddr.emitRawPointer(CGF), CGM.Int8PtrTy),
CGM.getSize(CGM.GetTargetTypeStoreSize(VarTy)),
getOrCreateThreadPrivateCache(VD)};
- return Address(
- CGF.EmitRuntimeCall(
- OMPBuilder.getOrCreateRuntimeFunction(
- CGM.getModule(), OMPRTL___kmpc_threadprivate_cached),
- Args),
- CGF.Int8Ty, VDAddr.getAlignment());
+ return Address(CGF.EmitRuntimeCall(
+ OMPBuilder.getOrCreateRuntimeFunction(
+ CGM.getModule(), OMPRTL___kmpc_threadprivate_cached),
+ Args),
+ CGF.Int8Ty, VDAddr.getAlignment());
}
void CGOpenMPRuntime::emitThreadPrivateVarInit(
@@ -1629,8 +1628,8 @@ void CGOpenMPRuntime::emitThreadPrivateVarInit(
}
llvm::Function *CGOpenMPRuntime::emitThreadPrivateVarDefinition(
-const VarDecl *VD, Address VDAddr, SourceLocation Loc,
-bool PerformInit, CodeGenFunction *CGF) {
+const VarDecl *VD, Address VDAddr, SourceLocation Loc, bool PerformInit,
+CodeGenFunction *CGF) {
if (CGM.getLangOpts().OpenMPUseTLS &&
CGM.getContext().getTargetInfo().isTLSSupported())
return nullptr;
@@ -1692,7 +1691,8 @@ llvm::Function
*CGOpenMPRuntime::emitThreadPrivateVarDefinition(
auto NL =
[clang] [llvm] [Clang][OpenMP] Non-contiguous strided update (PR #144635)
amitamd7 wrote: Please ignore the indentation changes. I'll fix them soon in the revised version along. https://github.com/llvm/llvm-project/pull/144635 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
