[clang] [OpenMP][libomptarget] Add map checks when running under unified shared memory (PR #69005)

2023-10-16 Thread Gheorghe-Teodor Bercea via cfe-commits

https://github.com/doru1004 updated 
https://github.com/llvm/llvm-project/pull/69005

>From cb4121c466a0fc357d6ca129bfdd4e7c5e2d11ee Mon Sep 17 00:00:00 2001
From: Doru Bercea 
Date: Wed, 16 Nov 2022 17:23:48 -0600
Subject: [PATCH 1/2] Fix declare target implementation to support enter.

---
 clang/include/clang/Basic/Attr.td |  4 +-
 .../clang/Basic/DiagnosticParseKinds.td   | 12 -
 clang/lib/AST/AttrImpl.cpp|  2 +-
 clang/lib/CodeGen/CGExpr.cpp  | 12 +++--
 clang/lib/CodeGen/CGOpenMPRuntime.cpp | 24 ++---
 clang/lib/CodeGen/CodeGenModule.cpp   |  6 ++-
 clang/lib/Parse/ParseOpenMP.cpp   | 39 ++
 clang/lib/Sema/SemaOpenMP.cpp | 10 ++--
 .../test/OpenMP/declare_target_ast_print.cpp  | 53 +++
 9 files changed, 130 insertions(+), 32 deletions(-)

diff --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 16cf932c3760bd3..eaf4a6db3600e07 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3749,8 +3749,8 @@ def OMPDeclareTargetDecl : InheritableAttr {
   let Documentation = [OMPDeclareTargetDocs];
   let Args = [
 EnumArgument<"MapType", "MapTypeTy",
- [ "to", "link" ],
- [ "MT_To", "MT_Link" ]>,
+ [ "to", "enter", "link" ],
+ [ "MT_To", "MT_Enter", "MT_Link" ]>,
 EnumArgument<"DevType", "DevTypeTy",
  [ "host", "nohost", "any" ],
  [ "DT_Host", "DT_NoHost", "DT_Any" ]>,
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 674d6bd34fc544f..27cd3da1f191c3d 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1383,12 +1383,22 @@ def note_omp_assumption_clause_continue_here
 : Note<"the ignored tokens spans until here">;
 def err_omp_declare_target_unexpected_clause: Error<
   "unexpected '%0' clause, only %select{'device_type'|'to' or 'link'|'to', 
'link' or 'device_type'|'device_type', 'indirect'|'to', 'link', 'device_type' 
or 'indirect'}1 clauses expected">;
+def err_omp_declare_target_unexpected_clause_52: Error<
+  "unexpected '%0' clause, only %select{'device_type'|'enter' or 
'link'|'enter', 'link' or 'device_type'|'device_type', 'indirect'|'enter', 
'link', 'device_type' or 'indirect'}1 clauses expected">;
 def err_omp_begin_declare_target_unexpected_implicit_to_clause: Error<
   "unexpected '(', only 'to', 'link' or 'device_type' clauses expected for 
'begin declare target' directive">;
-def err_omp_declare_target_unexpected_clause_after_implicit_to: Error<
+def err_omp_declare_target_wrong_clause_after_implicit_to: Error<
   "unexpected clause after an implicit 'to' clause">;
+def err_omp_declare_target_wrong_clause_after_implicit_enter: Error<
+  "unexpected clause after an implicit 'enter' clause">;
 def err_omp_declare_target_missing_to_or_link_clause: Error<
   "expected at least one %select{'to' or 'link'|'to', 'link' or 'indirect'}0 
clause">;
+def err_omp_declare_target_missing_enter_or_link_clause: Error<
+  "expected at least one %select{'enter' or 'link'|'enter', 'link' or 
'indirect'}0 clause">;
+def err_omp_declare_target_unexpected_to_clause: Error<
+  "unexpected 'to' clause, use 'enter' instead">;
+def err_omp_declare_target_unexpected_enter_clause: Error<
+  "unexpected 'enter' clause, use 'to' instead">;
 def err_omp_declare_target_multiple : Error<
   "%0 appears multiple times in clauses on the same declare target directive">;
 def err_omp_declare_target_indirect_device_type: Error<
diff --git a/clang/lib/AST/AttrImpl.cpp b/clang/lib/AST/AttrImpl.cpp
index cecbd703ac61e8c..da842f6b190e74d 100644
--- a/clang/lib/AST/AttrImpl.cpp
+++ b/clang/lib/AST/AttrImpl.cpp
@@ -137,7 +137,7 @@ void OMPDeclareTargetDeclAttr::printPrettyPragma(
   // Use fake syntax because it is for testing and debugging purpose only.
   if (getDevType() != DT_Any)
 OS << " device_type(" << ConvertDevTypeTyToStr(getDevType()) << ")";
-  if (getMapType() != MT_To)
+  if (getMapType() != MT_To && getMapType() != MT_Enter)
 OS << ' ' << ConvertMapTypeTyToStr(getMapType());
   if (Expr *E = getIndirectExpr()) {
 OS << " indirect(";
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index ee09a8566c3719e..77085ff34fca233 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -2495,14 +2495,16 @@ static Address 
emitDeclTargetVarDeclLValue(CodeGenFunction ,
const VarDecl *VD, QualType T) {
   llvm::Optional Res =
   OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD);
-  // Return an invalid address if variable is MT_To and unified
-  // memory is not enabled. For all other cases: MT_Link and
-  // MT_To with unified memory, return a valid address.
-  if (!Res || (*Res 

[clang] [OpenMP][libomptarget] Add map checks when running under unified shared memory (PR #69005)

2023-10-16 Thread Gheorghe-Teodor Bercea via cfe-commits

https://github.com/doru1004 updated 
https://github.com/llvm/llvm-project/pull/69005

>From cb4121c466a0fc357d6ca129bfdd4e7c5e2d11ee Mon Sep 17 00:00:00 2001
From: Doru Bercea 
Date: Wed, 16 Nov 2022 17:23:48 -0600
Subject: [PATCH 1/2] Fix declare target implementation to support enter.

---
 clang/include/clang/Basic/Attr.td |  4 +-
 .../clang/Basic/DiagnosticParseKinds.td   | 12 -
 clang/lib/AST/AttrImpl.cpp|  2 +-
 clang/lib/CodeGen/CGExpr.cpp  | 12 +++--
 clang/lib/CodeGen/CGOpenMPRuntime.cpp | 24 ++---
 clang/lib/CodeGen/CodeGenModule.cpp   |  6 ++-
 clang/lib/Parse/ParseOpenMP.cpp   | 39 ++
 clang/lib/Sema/SemaOpenMP.cpp | 10 ++--
 .../test/OpenMP/declare_target_ast_print.cpp  | 53 +++
 9 files changed, 130 insertions(+), 32 deletions(-)

diff --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 16cf932c3760bd3..eaf4a6db3600e07 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3749,8 +3749,8 @@ def OMPDeclareTargetDecl : InheritableAttr {
   let Documentation = [OMPDeclareTargetDocs];
   let Args = [
 EnumArgument<"MapType", "MapTypeTy",
- [ "to", "link" ],
- [ "MT_To", "MT_Link" ]>,
+ [ "to", "enter", "link" ],
+ [ "MT_To", "MT_Enter", "MT_Link" ]>,
 EnumArgument<"DevType", "DevTypeTy",
  [ "host", "nohost", "any" ],
  [ "DT_Host", "DT_NoHost", "DT_Any" ]>,
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 674d6bd34fc544f..27cd3da1f191c3d 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1383,12 +1383,22 @@ def note_omp_assumption_clause_continue_here
 : Note<"the ignored tokens spans until here">;
 def err_omp_declare_target_unexpected_clause: Error<
   "unexpected '%0' clause, only %select{'device_type'|'to' or 'link'|'to', 
'link' or 'device_type'|'device_type', 'indirect'|'to', 'link', 'device_type' 
or 'indirect'}1 clauses expected">;
+def err_omp_declare_target_unexpected_clause_52: Error<
+  "unexpected '%0' clause, only %select{'device_type'|'enter' or 
'link'|'enter', 'link' or 'device_type'|'device_type', 'indirect'|'enter', 
'link', 'device_type' or 'indirect'}1 clauses expected">;
 def err_omp_begin_declare_target_unexpected_implicit_to_clause: Error<
   "unexpected '(', only 'to', 'link' or 'device_type' clauses expected for 
'begin declare target' directive">;
-def err_omp_declare_target_unexpected_clause_after_implicit_to: Error<
+def err_omp_declare_target_wrong_clause_after_implicit_to: Error<
   "unexpected clause after an implicit 'to' clause">;
+def err_omp_declare_target_wrong_clause_after_implicit_enter: Error<
+  "unexpected clause after an implicit 'enter' clause">;
 def err_omp_declare_target_missing_to_or_link_clause: Error<
   "expected at least one %select{'to' or 'link'|'to', 'link' or 'indirect'}0 
clause">;
+def err_omp_declare_target_missing_enter_or_link_clause: Error<
+  "expected at least one %select{'enter' or 'link'|'enter', 'link' or 
'indirect'}0 clause">;
+def err_omp_declare_target_unexpected_to_clause: Error<
+  "unexpected 'to' clause, use 'enter' instead">;
+def err_omp_declare_target_unexpected_enter_clause: Error<
+  "unexpected 'enter' clause, use 'to' instead">;
 def err_omp_declare_target_multiple : Error<
   "%0 appears multiple times in clauses on the same declare target directive">;
 def err_omp_declare_target_indirect_device_type: Error<
diff --git a/clang/lib/AST/AttrImpl.cpp b/clang/lib/AST/AttrImpl.cpp
index cecbd703ac61e8c..da842f6b190e74d 100644
--- a/clang/lib/AST/AttrImpl.cpp
+++ b/clang/lib/AST/AttrImpl.cpp
@@ -137,7 +137,7 @@ void OMPDeclareTargetDeclAttr::printPrettyPragma(
   // Use fake syntax because it is for testing and debugging purpose only.
   if (getDevType() != DT_Any)
 OS << " device_type(" << ConvertDevTypeTyToStr(getDevType()) << ")";
-  if (getMapType() != MT_To)
+  if (getMapType() != MT_To && getMapType() != MT_Enter)
 OS << ' ' << ConvertMapTypeTyToStr(getMapType());
   if (Expr *E = getIndirectExpr()) {
 OS << " indirect(";
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index ee09a8566c3719e..77085ff34fca233 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -2495,14 +2495,16 @@ static Address 
emitDeclTargetVarDeclLValue(CodeGenFunction ,
const VarDecl *VD, QualType T) {
   llvm::Optional Res =
   OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD);
-  // Return an invalid address if variable is MT_To and unified
-  // memory is not enabled. For all other cases: MT_Link and
-  // MT_To with unified memory, return a valid address.
-  if (!Res || (*Res 

[clang] [OpenMP][libomptarget] Add map checks when running under unified shared memory (PR #69005)

2023-10-16 Thread Gheorghe-Teodor Bercea via cfe-commits

https://github.com/doru1004 updated 
https://github.com/llvm/llvm-project/pull/69005

>From cb4121c466a0fc357d6ca129bfdd4e7c5e2d11ee Mon Sep 17 00:00:00 2001
From: Doru Bercea 
Date: Wed, 16 Nov 2022 17:23:48 -0600
Subject: [PATCH 1/2] Fix declare target implementation to support enter.

---
 clang/include/clang/Basic/Attr.td |  4 +-
 .../clang/Basic/DiagnosticParseKinds.td   | 12 -
 clang/lib/AST/AttrImpl.cpp|  2 +-
 clang/lib/CodeGen/CGExpr.cpp  | 12 +++--
 clang/lib/CodeGen/CGOpenMPRuntime.cpp | 24 ++---
 clang/lib/CodeGen/CodeGenModule.cpp   |  6 ++-
 clang/lib/Parse/ParseOpenMP.cpp   | 39 ++
 clang/lib/Sema/SemaOpenMP.cpp | 10 ++--
 .../test/OpenMP/declare_target_ast_print.cpp  | 53 +++
 9 files changed, 130 insertions(+), 32 deletions(-)

diff --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 16cf932c3760bd3..eaf4a6db3600e07 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3749,8 +3749,8 @@ def OMPDeclareTargetDecl : InheritableAttr {
   let Documentation = [OMPDeclareTargetDocs];
   let Args = [
 EnumArgument<"MapType", "MapTypeTy",
- [ "to", "link" ],
- [ "MT_To", "MT_Link" ]>,
+ [ "to", "enter", "link" ],
+ [ "MT_To", "MT_Enter", "MT_Link" ]>,
 EnumArgument<"DevType", "DevTypeTy",
  [ "host", "nohost", "any" ],
  [ "DT_Host", "DT_NoHost", "DT_Any" ]>,
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 674d6bd34fc544f..27cd3da1f191c3d 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1383,12 +1383,22 @@ def note_omp_assumption_clause_continue_here
 : Note<"the ignored tokens spans until here">;
 def err_omp_declare_target_unexpected_clause: Error<
   "unexpected '%0' clause, only %select{'device_type'|'to' or 'link'|'to', 
'link' or 'device_type'|'device_type', 'indirect'|'to', 'link', 'device_type' 
or 'indirect'}1 clauses expected">;
+def err_omp_declare_target_unexpected_clause_52: Error<
+  "unexpected '%0' clause, only %select{'device_type'|'enter' or 
'link'|'enter', 'link' or 'device_type'|'device_type', 'indirect'|'enter', 
'link', 'device_type' or 'indirect'}1 clauses expected">;
 def err_omp_begin_declare_target_unexpected_implicit_to_clause: Error<
   "unexpected '(', only 'to', 'link' or 'device_type' clauses expected for 
'begin declare target' directive">;
-def err_omp_declare_target_unexpected_clause_after_implicit_to: Error<
+def err_omp_declare_target_wrong_clause_after_implicit_to: Error<
   "unexpected clause after an implicit 'to' clause">;
+def err_omp_declare_target_wrong_clause_after_implicit_enter: Error<
+  "unexpected clause after an implicit 'enter' clause">;
 def err_omp_declare_target_missing_to_or_link_clause: Error<
   "expected at least one %select{'to' or 'link'|'to', 'link' or 'indirect'}0 
clause">;
+def err_omp_declare_target_missing_enter_or_link_clause: Error<
+  "expected at least one %select{'enter' or 'link'|'enter', 'link' or 
'indirect'}0 clause">;
+def err_omp_declare_target_unexpected_to_clause: Error<
+  "unexpected 'to' clause, use 'enter' instead">;
+def err_omp_declare_target_unexpected_enter_clause: Error<
+  "unexpected 'enter' clause, use 'to' instead">;
 def err_omp_declare_target_multiple : Error<
   "%0 appears multiple times in clauses on the same declare target directive">;
 def err_omp_declare_target_indirect_device_type: Error<
diff --git a/clang/lib/AST/AttrImpl.cpp b/clang/lib/AST/AttrImpl.cpp
index cecbd703ac61e8c..da842f6b190e74d 100644
--- a/clang/lib/AST/AttrImpl.cpp
+++ b/clang/lib/AST/AttrImpl.cpp
@@ -137,7 +137,7 @@ void OMPDeclareTargetDeclAttr::printPrettyPragma(
   // Use fake syntax because it is for testing and debugging purpose only.
   if (getDevType() != DT_Any)
 OS << " device_type(" << ConvertDevTypeTyToStr(getDevType()) << ")";
-  if (getMapType() != MT_To)
+  if (getMapType() != MT_To && getMapType() != MT_Enter)
 OS << ' ' << ConvertMapTypeTyToStr(getMapType());
   if (Expr *E = getIndirectExpr()) {
 OS << " indirect(";
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index ee09a8566c3719e..77085ff34fca233 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -2495,14 +2495,16 @@ static Address 
emitDeclTargetVarDeclLValue(CodeGenFunction ,
const VarDecl *VD, QualType T) {
   llvm::Optional Res =
   OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD);
-  // Return an invalid address if variable is MT_To and unified
-  // memory is not enabled. For all other cases: MT_Link and
-  // MT_To with unified memory, return a valid address.
-  if (!Res || (*Res 

[clang] [OpenMP][libomptarget] Add map checks when running under unified shared memory (PR #69005)

2023-10-16 Thread Gheorghe-Teodor Bercea via cfe-commits


@@ -444,6 +486,29 @@ DeviceTy::getTgtPtrBegin(void *HstPtrBegin, int64_t Size, 
bool UpdateRefCount,
  LR.TPR.getEntry()->dynRefCountToStr().c_str(), DynRefCountAction,
  LR.TPR.getEntry()->holdRefCountToStr().c_str(), HoldRefCountAction);
 LR.TPR.TargetPointer = (void *)TP;
+
+// If this entry is not marked as being host pointer (the way the
+// implementation works today this is never true, mistake?) then we
+// have to check if this is a host pointer or not. This is a host pointer
+// if the host address matches the target address.
+if ((PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY) &&
+!LR.TPR.Flags.IsHostPointer) {

doru1004 wrote:

There are several tests which exercise the call to the getTgtPtrBegin. The 
reason this change is needed is because the first condition, if true, and it 
can be true even when USM is enabled, then the USM branch will not be taken at 
all and the IsHostPointer and IsPresent will not be correctly set.

https://github.com/llvm/llvm-project/pull/69005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [OpenMP][libomptarget] Add map checks when running under unified shared memory (PR #69005)

2023-10-13 Thread via cfe-commits


@@ -289,13 +306,38 @@ TargetPointerResultTy DeviceTy::getTargetPointer(
 // In addition to the mapping rules above, the close map modifier forces 
the
 // mapping of the variable to the device.
 if (Size) {
-  DP("Return HstPtrBegin " DPxMOD " Size=%" PRId64 " for unified shared "
- "memory\n",
- DPxPTR((uintptr_t)HstPtrBegin), Size);
-  LR.TPR.Flags.IsPresent = false;
+  LR.TPR.Flags.IsNewEntry = true;
+  assert(TgtPadding == 0 && "TgtPadding must always be zero in USM mode");
+  uintptr_t TgtPtrBegin = (uintptr_t)HstPtrBegin + TgtPadding;
+  LR.TPR.setEntry(
+  HDTTMap
+  ->emplace(new HostDataToTargetTy(
+  (uintptr_t)HstPtrBase, (uintptr_t)HstPtrBegin,
+  (uintptr_t)HstPtrBegin + Size, (uintptr_t)HstPtrBegin,
+  TgtPtrBegin, HasHoldModifier, HstPtrName))
+  .first->HDTT);
+  INFO(OMP_INFOTYPE_MAPPING_CHANGED, DeviceID,
+   "Creating new map entry ONLY with HstPtrBase=" DPxMOD
+   ", HstPtrBegin=" DPxMOD ", TgtAllocBegin=" DPxMOD
+   ", TgtPtrBegin=" DPxMOD
+   ", Size=%ld, DynRefCount=%s, HoldRefCount=%s, Name=%s\n",
+   DPxPTR(HstPtrBase), DPxPTR(HstPtrBegin), DPxPTR(HstPtrBegin),
+   DPxPTR(TgtPtrBegin), Size,
+   LR.TPR.getEntry()->dynRefCountToStr().c_str(),
+   LR.TPR.getEntry()->holdRefCountToStr().c_str(),
+   (HstPtrName) ? getNameFromMapping(HstPtrName).c_str() : "unknown");
   LR.TPR.Flags.IsHostPointer = true;
+
+  // The following assert should catch any case in which the pointers
+  // do not match to understand if this case can ever happen.
+  assert((uintptr_t)HstPtrBegin == TgtPtrBegin &&
+ "Pointers must always match");
+
+  // If the above assert is ever hit the following should be changed to =
+  // TgtPtrBegin
   LR.TPR.TargetPointer = HstPtrBegin;
 }
+LR.TPR.Flags.IsPresent = false;

carlobertolli wrote:

How do we implement omp_target_is_present()?
If we implement it by checking if the input pointer is contained within 
previously mapped memory, then we might want to change this to "true".

Here's a reminder of the definition of the API above:

> The omp_target_is_present routine tests whether a host pointer refers to 
> storage that is
> mapped to a given device.

Under unified_shared_memory, mapping memory should mean the same as in non 
unified_shared_memory: not that memory is being allocated and copied on some 
physical storage device, but that the device data environment contains that 
memory, which in this case it does (and it is accessed via the host pointer).
I believe omp_target_is_present implementation has been discussed in the 
context of OpenMP acceleration subcommittee, so we may want to check on what 
TR11 and any "in fieri" document requires about this.

https://github.com/llvm/llvm-project/pull/69005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [OpenMP][libomptarget] Add map checks when running under unified shared memory (PR #69005)

2023-10-13 Thread via cfe-commits


@@ -0,0 +1,158 @@
+// RUN: %libomptarget-compilexx-generic && env HSA_XNACK=1 LIBOMPTARGET_INFO=-1
+// %libomptarget-run-generic 2>&1 | %fcheck-generic
+
+// UNSUPPORTED: clang-6, clang-7, clang-8, clang-9
+
+// REQUIRES: amdgcn-amd-amdhsa
+
+#include 
+#include 
+
+#pragma omp requires unified_shared_memory
+
+#define N 1024
+
+int main(int argc, char *argv[]) {
+  int fails;
+  void *host_alloc, *device_alloc;
+  void *host_data, *device_data;
+  int *alloc = (int *)malloc(N * sizeof(int));
+  int data[N];
+
+  for (int i = 0; i < N; ++i) {
+alloc[i] = 10;
+data[i] = 1;
+  }
+
+  host_data = [0];
+  host_alloc = [0];
+
+// CHECK: Creating new map entry ONLY with
+// HstPtrBase=[[DEVICE_DATA_HST_PTR:0x.*]], 
HstPtrBegin=[[DEVICE_DATA_HST_PTR]],
+// TgtAllocBegin=[[DEVICE_DATA_HST_PTR]], TgtPtrBegin=[[DEVICE_DATA_HST_PTR]],
+// Size=8, DynRefCount=1, HoldRefCount=0 CHECK: Creating new map entry ONLY 
with
+// HstPtrBase=[[DATA_HST_PTR:0x.*]], HstPtrBegin=[[DATA_HST_PTR]],
+// TgtAllocBegin=[[DATA_HST_PTR]], TgtPtrBegin=[[DATA_HST_PTR]], Size=4096,
+// DynRefCount=1, HoldRefCount=0 CHECK: Creating new map entry ONLY with
+// HstPtrBase=[[DEVICE_ALLOC_HST_PTR:0x.*]],
+// HstPtrBegin=[[DEVICE_ALLOC_HST_PTR]], 
TgtAllocBegin=[[DEVICE_ALLOC_HST_PTR]],
+// TgtPtrBegin=[[DEVICE_ALLOC_HST_PTR]], Size=8, DynRefCount=1, HoldRefCount=0
+
+// CHECK: Mapping exists with HstPtrBegin=[[DEVICE_DATA_HST_PTR]],
+// TgtPtrBegin=[[DEVICE_DATA_HST_PTR]], Size=8, DynRefCount=1 (update
+// suppressed), HoldRefCount=0 CHECK: Mapping exists with
+// HstPtrBegin=[[DATA_HST_PTR]], TgtPtrBegin=[[DATA_HST_PTR]], Size=4096,
+// DynRefCount=1 (update suppressed), HoldRefCount=0 CHECK: Mapping exists with
+// HstPtrBegin=[[DEVICE_ALLOC_HST_PTR]], TgtPtrBegin=[[DEVICE_ALLOC_HST_PTR]],
+// Size=8, DynRefCount=1 (update suppressed), HoldRefCount=0
+
+// CHECK: Launching kernel __omp_offloading_{{.*}}_main_l{{.*}} with 1 blocks
+// and 256 threads in Generic mode
+
+// CHECK: Mapping exists with HstPtrBegin=[[DEVICE_ALLOC_HST_PTR]],
+// TgtPtrBegin=[[DEVICE_ALLOC_HST_PTR]], Size=8, DynRefCount=0 (decremented,
+// delayed deletion), HoldRefCount=0 CHECK: Mapping exists with
+// HstPtrBegin=[[DATA_HST_PTR]], TgtPtrBegin=[[DATA_HST_PTR]], Size=4096,
+// DynRefCount=0 (decremented, delayed deletion), HoldRefCount=0 CHECK: Mapping
+// exists with HstPtrBegin=[[DEVICE_DATA_HST_PTR]],
+// TgtPtrBegin=[[DEVICE_DATA_HST_PTR]], Size=8, DynRefCount=0 (decremented,
+// delayed deletion), HoldRefCount=0
+
+// CHECK: Removing map entry with HstPtrBegin=[[DEVICE_ALLOC_HST_PTR]]{{.*}}
+// Size=8 CHECK: Removing map entry with HstPtrBegin=[[DATA_HST_PTR]]{{.*}}
+// Size=4096 CHECK: Removing map entry with
+// HstPtrBegin=[[DEVICE_DATA_HST_PTR]]{{.*}} Size=8
+
+// implicit mapping of data
+#pragma omp target map(tofrom : device_data, device_alloc)
+  {
+device_data = [0];
+device_alloc = [0];
+
+for (int i = 0; i < N; i++) {
+  alloc[i] += 1;
+  data[i] += 1;
+}
+  }
+
+  if (device_alloc == host_alloc)
+printf("Address of alloc on device matches host address.\n");
+
+  if (device_data == host_data)
+printf("Address of data on device matches host address.\n");
+
+  // On the host, check that the arrays have been updated.
+  fails = 0;
+  for (int i = 0; i < N; i++) {
+if (alloc[i] != 11)
+  fails++;
+  }
+  printf("Alloc device values updated: %s\n",
+ (fails == 0) ? "Succeeded" : "Failed");
+
+  fails = 0;
+  for (int i = 0; i < N; i++) {
+if (data[i] != 2)
+  fails++;
+  }
+  printf("Data device values updated: %s\n",
+ (fails == 0) ? "Succeeded" : "Failed");
+
+  //
+  // Test that updates on the host snd on the device are both visible.

carlobertolli wrote:

snd-->and

https://github.com/llvm/llvm-project/pull/69005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [OpenMP][libomptarget] Add map checks when running under unified shared memory (PR #69005)

2023-10-13 Thread via cfe-commits


@@ -268,6 +268,23 @@ TargetPointerResultTy DeviceTy::getTargetPointer(
  LR.TPR.getEntry()->holdRefCountToStr().c_str(), HoldRefCountAction,
  (HstPtrName) ? getNameFromMapping(HstPtrName).c_str() : "unknown");
 LR.TPR.TargetPointer = (void *)Ptr;
+
+if (PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY &&
+!HasCloseModifier && !LR.TPR.Flags.IsHostPointer) {
+  // This is a host pointer and is not present if the pointers match:
+  if (LR.TPR.getEntry()->TgtPtrBegin == LR.TPR.getEntry()->HstPtrBegin) {
+LR.TPR.Flags.IsPresent = false;
+LR.TPR.Flags.IsHostPointer = true;
+  }
+
+  // Catch the case where incmoing HstPtrBegin is not consistent with the

carlobertolli wrote:

incmoing->incoming

https://github.com/llvm/llvm-project/pull/69005
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [OpenMP][libomptarget] Add map checks when running under unified shared memory (PR #69005)

2023-10-13 Thread Gheorghe-Teodor Bercea via cfe-commits

https://github.com/doru1004 updated 
https://github.com/llvm/llvm-project/pull/69005

>From cb4121c466a0fc357d6ca129bfdd4e7c5e2d11ee Mon Sep 17 00:00:00 2001
From: Doru Bercea 
Date: Wed, 16 Nov 2022 17:23:48 -0600
Subject: [PATCH 1/2] Fix declare target implementation to support enter.

---
 clang/include/clang/Basic/Attr.td |  4 +-
 .../clang/Basic/DiagnosticParseKinds.td   | 12 -
 clang/lib/AST/AttrImpl.cpp|  2 +-
 clang/lib/CodeGen/CGExpr.cpp  | 12 +++--
 clang/lib/CodeGen/CGOpenMPRuntime.cpp | 24 ++---
 clang/lib/CodeGen/CodeGenModule.cpp   |  6 ++-
 clang/lib/Parse/ParseOpenMP.cpp   | 39 ++
 clang/lib/Sema/SemaOpenMP.cpp | 10 ++--
 .../test/OpenMP/declare_target_ast_print.cpp  | 53 +++
 9 files changed, 130 insertions(+), 32 deletions(-)

diff --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 16cf932c3760bd3..eaf4a6db3600e07 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -3749,8 +3749,8 @@ def OMPDeclareTargetDecl : InheritableAttr {
   let Documentation = [OMPDeclareTargetDocs];
   let Args = [
 EnumArgument<"MapType", "MapTypeTy",
- [ "to", "link" ],
- [ "MT_To", "MT_Link" ]>,
+ [ "to", "enter", "link" ],
+ [ "MT_To", "MT_Enter", "MT_Link" ]>,
 EnumArgument<"DevType", "DevTypeTy",
  [ "host", "nohost", "any" ],
  [ "DT_Host", "DT_NoHost", "DT_Any" ]>,
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td 
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 674d6bd34fc544f..27cd3da1f191c3d 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1383,12 +1383,22 @@ def note_omp_assumption_clause_continue_here
 : Note<"the ignored tokens spans until here">;
 def err_omp_declare_target_unexpected_clause: Error<
   "unexpected '%0' clause, only %select{'device_type'|'to' or 'link'|'to', 
'link' or 'device_type'|'device_type', 'indirect'|'to', 'link', 'device_type' 
or 'indirect'}1 clauses expected">;
+def err_omp_declare_target_unexpected_clause_52: Error<
+  "unexpected '%0' clause, only %select{'device_type'|'enter' or 
'link'|'enter', 'link' or 'device_type'|'device_type', 'indirect'|'enter', 
'link', 'device_type' or 'indirect'}1 clauses expected">;
 def err_omp_begin_declare_target_unexpected_implicit_to_clause: Error<
   "unexpected '(', only 'to', 'link' or 'device_type' clauses expected for 
'begin declare target' directive">;
-def err_omp_declare_target_unexpected_clause_after_implicit_to: Error<
+def err_omp_declare_target_wrong_clause_after_implicit_to: Error<
   "unexpected clause after an implicit 'to' clause">;
+def err_omp_declare_target_wrong_clause_after_implicit_enter: Error<
+  "unexpected clause after an implicit 'enter' clause">;
 def err_omp_declare_target_missing_to_or_link_clause: Error<
   "expected at least one %select{'to' or 'link'|'to', 'link' or 'indirect'}0 
clause">;
+def err_omp_declare_target_missing_enter_or_link_clause: Error<
+  "expected at least one %select{'enter' or 'link'|'enter', 'link' or 
'indirect'}0 clause">;
+def err_omp_declare_target_unexpected_to_clause: Error<
+  "unexpected 'to' clause, use 'enter' instead">;
+def err_omp_declare_target_unexpected_enter_clause: Error<
+  "unexpected 'enter' clause, use 'to' instead">;
 def err_omp_declare_target_multiple : Error<
   "%0 appears multiple times in clauses on the same declare target directive">;
 def err_omp_declare_target_indirect_device_type: Error<
diff --git a/clang/lib/AST/AttrImpl.cpp b/clang/lib/AST/AttrImpl.cpp
index cecbd703ac61e8c..da842f6b190e74d 100644
--- a/clang/lib/AST/AttrImpl.cpp
+++ b/clang/lib/AST/AttrImpl.cpp
@@ -137,7 +137,7 @@ void OMPDeclareTargetDeclAttr::printPrettyPragma(
   // Use fake syntax because it is for testing and debugging purpose only.
   if (getDevType() != DT_Any)
 OS << " device_type(" << ConvertDevTypeTyToStr(getDevType()) << ")";
-  if (getMapType() != MT_To)
+  if (getMapType() != MT_To && getMapType() != MT_Enter)
 OS << ' ' << ConvertMapTypeTyToStr(getMapType());
   if (Expr *E = getIndirectExpr()) {
 OS << " indirect(";
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index ee09a8566c3719e..77085ff34fca233 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -2495,14 +2495,16 @@ static Address 
emitDeclTargetVarDeclLValue(CodeGenFunction ,
const VarDecl *VD, QualType T) {
   llvm::Optional Res =
   OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD);
-  // Return an invalid address if variable is MT_To and unified
-  // memory is not enabled. For all other cases: MT_Link and
-  // MT_To with unified memory, return a valid address.
-  if (!Res || (*Res