jyu2 created this revision.
jyu2 added reviewers: ABataev, mikerice, jdoerfert.
jyu2 added a project: OpenMP.
Herald added a project: All.
jyu2 requested review of this revision.
Herald added subscribers: openmp-commits, cfe-commits, sstefan1.
Herald added a project: clang.

This is to fix run time problem when use:

int *a[3];
map((*a)[:3]) or (*a)[1].

current we skip generate map info for dereference pointer:
&(*a), &(*a)[0], 3*sizeof(int), TARGET_PARAM | TO | FROM

One way to fix runtime problem is to generate map info for dereference
pointer.

&(*a), &(*a), sizeof(pointer), ALLOC | TARGET_PARAM
&(*a), &(*a)[0], 3*sizeof(int),  PTR_AND_OBJ | TO | FROM

The change in CGOpenMPRuntime.cpp add that.

The change in SemaOpenMP is to fix variable of dereference array captured
by reference.  That is wrong. That cause run time to fail.

The rule is:
If variable is identified in a map clause it is always captured by
reference except if it is a pointer that is dereferenced somehow.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145093

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_map_deref_array_codegen.cpp
  openmp/libomptarget/test/mapping/target_derefence_array_pointrs.cpp

Index: openmp/libomptarget/test/mapping/target_derefence_array_pointrs.cpp
===================================================================
--- /dev/null
+++ openmp/libomptarget/test/mapping/target_derefence_array_pointrs.cpp
@@ -0,0 +1,33 @@
+// RUN: %libomptarget-compilexx-generic -fopenmp-version=51
+// RUN: %libomptarget-run-generic 2>&1 \
+// RUN: | %fcheck-generic
+
+#include <stdio.h>
+#include <stdlib.h>
+
+void foo(int **t1d) {
+  int ***t2d = &t1d;
+  int ****t3d = &t2d;
+  *t1d = (int *)malloc(3 * sizeof(int));
+  int j;
+
+  for (j = 0; j < 3; j++)
+    (*t1d)[j] = 0;
+#pragma omp target map(tofrom : (*t1d)[0 : 3])
+  { (*t1d)[1] = 1; }
+  // CHECK: 1
+  printf("%d\n", (*t1d)[1]);
+#pragma omp target map(tofrom : (**t2d)[0 : 3])
+  { (**t2d)[1] = 2; }
+  // CHECK: 2
+  printf("%d\n", (**t2d)[1]);
+#pragma omp target map(tofrom : (***t3d)[0 : 3])
+  { (***t3d)[1] = 3; }
+  // CHECK: 3
+  printf("%d\n", (***t3d)[1]);
+}
+
+int main() {
+  int *data = 0;
+  foo(&data);
+}
Index: clang/test/OpenMP/target_map_deref_array_codegen.cpp
===================================================================
--- /dev/null
+++ clang/test/OpenMP/target_map_deref_array_codegen.cpp
@@ -0,0 +1,131 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --function-signature --include-generated-funcs --replace-value-regex "__omp_offloading_[0-9a-z]+_[0-9a-z]+" "reduction_size[.].+[.]" "pl_cond[.].+[.|,]" --prefix-filecheck-ir-name _
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=powerpc64le-ibm-linux-gnu -x c++ -triple powerpc64le-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+extern void *malloc (int __size) throw () __attribute__ ((__malloc__));
+
+void foo(int **t1d)
+{
+  *t1d = (int *) malloc(3 * sizeof(int));
+  for (int j=0; j < 3; j++)
+    (*t1d)[j] = 1;
+  #pragma omp target map(to: (*t1d)[0:3])
+    (*t1d)[2] = 2;
+}
+
+#endif
+
+// CHECK: @.offload_sizes = private unnamed_addr constant [2 x i64] [i64 8, i64 12]
+// CHECK: @.offload_maptypes = private unnamed_addr constant [2 x i64] [i64 32, i64 17]
+// CHECK-LABEL: define {{[^@]+}}@_Z3fooPPi
+// CHECK-SAME: (ptr noundef [[T1D:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[T1D_ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:    [[J:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    [[DOTOFFLOAD_BASEPTRS:%.*]] = alloca [2 x ptr], align 8
+// CHECK-NEXT:    [[DOTOFFLOAD_PTRS:%.*]] = alloca [2 x ptr], align 8
+// CHECK-NEXT:    [[DOTOFFLOAD_MAPPERS:%.*]] = alloca [2 x ptr], align 8
+// CHECK-NEXT:    store ptr [[T1D]], ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:    [[CALL:%.*]] = call noalias noundef ptr @_Z6malloci(i32 noundef signext 12) #[[ATTR3:[0-9]+]]
+// CHECK-NEXT:    [[TMP0:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:    store ptr [[CALL]], ptr [[TMP0]], align 8
+// CHECK-NEXT:    store i32 0, ptr [[J]], align 4
+// CHECK-NEXT:    br label [[FOR_COND:%.*]]
+// CHECK:       for.cond:
+// CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr [[J]], align 4
+// CHECK-NEXT:    [[CMP:%.*]] = icmp slt i32 [[TMP1]], 3
+// CHECK-NEXT:    br i1 [[CMP]], label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
+// CHECK:       for.body:
+// CHECK-NEXT:    [[TMP2:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:    [[TMP3:%.*]] = load ptr, ptr [[TMP2]], align 8
+// CHECK-NEXT:    [[TMP4:%.*]] = load i32, ptr [[J]], align 4
+// CHECK-NEXT:    [[IDXPROM:%.*]] = sext i32 [[TMP4]] to i64
+// CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[TMP3]], i64 [[IDXPROM]]
+// CHECK-NEXT:    store i32 1, ptr [[ARRAYIDX]], align 4
+// CHECK-NEXT:    br label [[FOR_INC:%.*]]
+// CHECK:       for.inc:
+// CHECK-NEXT:    [[TMP5:%.*]] = load i32, ptr [[J]], align 4
+// CHECK-NEXT:    [[INC:%.*]] = add nsw i32 [[TMP5]], 1
+// CHECK-NEXT:    store i32 [[INC]], ptr [[J]], align 4
+// CHECK-NEXT:    br label [[FOR_COND]], !llvm.loop [[LOOP4:![0-9]+]]
+// CHECK:       for.end:
+// CHECK-NEXT:    [[TMP6:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:    [[TMP7:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:    [[TMP8:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:    [[TMP9:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:    [[TMP10:%.*]] = load ptr, ptr [[TMP9]], align 8
+// CHECK-NEXT:    [[ARRAYIDX1:%.*]] = getelementptr inbounds i32, ptr [[TMP10]], i64 0
+// CHECK-NEXT:    [[TMP11:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_BASEPTRS]], i32 0, i32 0
+// CHECK-NEXT:    store ptr [[TMP7]], ptr [[TMP11]], align 8
+// CHECK-NEXT:    [[TMP12:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_PTRS]], i32 0, i32 0
+// CHECK-NEXT:    store ptr [[TMP8]], ptr [[TMP12]], align 8
+// CHECK-NEXT:    [[TMP13:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_MAPPERS]], i64 0, i64 0
+// CHECK-NEXT:    store ptr null, ptr [[TMP13]], align 8
+// CHECK-NEXT:    [[TMP14:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_BASEPTRS]], i32 0, i32 1
+// CHECK-NEXT:    store ptr [[TMP8]], ptr [[TMP14]], align 8
+// CHECK-NEXT:    [[TMP15:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_PTRS]], i32 0, i32 1
+// CHECK-NEXT:    store ptr [[ARRAYIDX1]], ptr [[TMP15]], align 8
+// CHECK-NEXT:    [[TMP16:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_MAPPERS]], i64 0, i64 1
+// CHECK-NEXT:    store ptr null, ptr [[TMP16]], align 8
+// CHECK-NEXT:    [[TMP17:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_BASEPTRS]], i32 0, i32 0
+// CHECK-NEXT:    [[TMP18:%.*]] = getelementptr inbounds [2 x ptr], ptr [[DOTOFFLOAD_PTRS]], i32 0, i32 0
+// CHECK-NEXT:    [[KERNEL_ARGS:%.*]] = alloca [[STRUCT___TGT_KERNEL_ARGUMENTS:%.*]], align 8
+// CHECK-NEXT:    [[TMP19:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 0
+// CHECK-NEXT:    store i32 2, ptr [[TMP19]], align 4
+// CHECK-NEXT:    [[TMP20:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 1
+// CHECK-NEXT:    store i32 2, ptr [[TMP20]], align 4
+// CHECK-NEXT:    [[TMP21:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 2
+// CHECK-NEXT:    store ptr [[TMP17]], ptr [[TMP21]], align 8
+// CHECK-NEXT:    [[TMP22:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 3
+// CHECK-NEXT:    store ptr [[TMP18]], ptr [[TMP22]], align 8
+// CHECK-NEXT:    [[TMP23:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 4
+// CHECK-NEXT:    store ptr @.offload_sizes, ptr [[TMP23]], align 8
+// CHECK-NEXT:    [[TMP24:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 5
+// CHECK-NEXT:    store ptr @.offload_maptypes, ptr [[TMP24]], align 8
+// CHECK-NEXT:    [[TMP25:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 6
+// CHECK-NEXT:    store ptr null, ptr [[TMP25]], align 8
+// CHECK-NEXT:    [[TMP26:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 7
+// CHECK-NEXT:    store ptr null, ptr [[TMP26]], align 8
+// CHECK-NEXT:    [[TMP27:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 8
+// CHECK-NEXT:    store i64 0, ptr [[TMP27]], align 8
+// CHECK-NEXT:    [[TMP28:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 9
+// CHECK-NEXT:    store i64 0, ptr [[TMP28]], align 8
+// CHECK-NEXT:    [[TMP29:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 10
+// CHECK-NEXT:    store [3 x i32] [i32 -1, i32 0, i32 0], ptr [[TMP29]], align 4
+// CHECK-NEXT:    [[TMP30:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 11
+// CHECK-NEXT:    store [3 x i32] zeroinitializer, ptr [[TMP30]], align 4
+// CHECK-NEXT:    [[TMP31:%.*]] = getelementptr inbounds [[STRUCT___TGT_KERNEL_ARGUMENTS]], ptr [[KERNEL_ARGS]], i32 0, i32 12
+// CHECK-NEXT:    store i32 0, ptr [[TMP31]], align 4
+// CHECK-NEXT:    [[TMP32:%.*]] = call i32 @__tgt_target_kernel(ptr @[[GLOB1:[0-9]+]], i64 -1, i32 -1, i32 0, ptr @.{{__omp_offloading_[0-9a-z]+_[0-9a-z]+}}__Z3fooPPi_l17.region_id, ptr [[KERNEL_ARGS]])
+// CHECK-NEXT:    [[TMP33:%.*]] = icmp ne i32 [[TMP32]], 0
+// CHECK-NEXT:    br i1 [[TMP33]], label [[OMP_OFFLOAD_FAILED:%.*]], label [[OMP_OFFLOAD_CONT:%.*]]
+// CHECK:       omp_offload.failed:
+// CHECK-NEXT:    call void @{{__omp_offloading_[0-9a-z]+_[0-9a-z]+}}__Z3fooPPi_l17(ptr [[TMP6]]) #[[ATTR3]]
+// CHECK-NEXT:    br label [[OMP_OFFLOAD_CONT]]
+// CHECK:       omp_offload.cont:
+// CHECK-NEXT:    ret void
+//
+//
+// CHECK-LABEL: define {{[^@]+}}@{{__omp_offloading_[0-9a-z]+_[0-9a-z]+}}__Z3fooPPi_l17
+// CHECK-SAME: (ptr noundef [[T1D:%.*]]) #[[ATTR2:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[T1D_ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:    store ptr [[T1D]], ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:    [[TMP0:%.*]] = load ptr, ptr [[T1D_ADDR]], align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = load ptr, ptr [[TMP0]], align 8
+// CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds i32, ptr [[TMP1]], i64 2
+// CHECK-NEXT:    store i32 2, ptr [[ARRAYIDX]], align 4
+// CHECK-NEXT:    ret void
+//
+//
+// CHECK-LABEL: define {{[^@]+}}@.omp_offloading.requires_reg
+// CHECK-SAME: () #[[ATTR4:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    call void @__tgt_register_requires(i64 1)
+// CHECK-NEXT:    ret void
+//
Index: clang/lib/Sema/SemaOpenMP.cpp
===================================================================
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2203,14 +2203,16 @@
           ++EI;
           if (EI == EE)
             return false;
-
-          if (isa<ArraySubscriptExpr>(EI->getAssociatedExpression()) ||
-              isa<OMPArraySectionExpr>(EI->getAssociatedExpression()) ||
-              isa<MemberExpr>(EI->getAssociatedExpression()) ||
-              isa<OMPArrayShapingExpr>(EI->getAssociatedExpression())) {
-            IsVariableAssociatedWithSection = true;
-            // There is nothing more we need to know about this variable.
-            return true;
+          auto Next = EI;
+          for (; Next != EE; Next = std::next(Next)) {
+            if (isa<ArraySubscriptExpr>(Next->getAssociatedExpression()) ||
+                isa<OMPArraySectionExpr>(Next->getAssociatedExpression()) ||
+                isa<MemberExpr>(EI->getAssociatedExpression()) ||
+                isa<OMPArrayShapingExpr>(Next->getAssociatedExpression())) {
+              IsVariableAssociatedWithSection = true;
+              // There is nothing more we need to know about this variable.
+              return true;
+            }
           }
 
           // Keep looking for more map info.
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===================================================================
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -7163,6 +7163,7 @@
     // double d;
     // int i[100];
     // float *p;
+    // int **a = &i;
     //
     // struct S1 {
     //   int i;
@@ -7196,6 +7197,10 @@
     // in unified shared memory mode or for local pointers
     // p, &p[1], 24*sizeof(float), TARGET_PARAM | TO | FROM
     //
+    // map((*a)[0:3])
+    // &(*a), &(*a), sizeof(pointer), ALLOC | TARGET_PARAM
+    // &(*a), &(*a)[0], 3*sizeof(int), PTR_AND_OBJ | TO | FROM
+    //
     // map(s)
     // &s, &s, sizeof(S2), TARGET_PARAM | TO | FROM
     //
@@ -7426,6 +7431,7 @@
 
     bool IsNonContiguous = CombinedInfo.NonContigInfo.IsNonContiguous;
     bool IsPrevMemberReference = false;
+    OpenMPMapClauseKind SaveMapType = MapType;
 
     for (; I != CE; ++I) {
       // If the current component is member of a struct (parent struct) mark it.
@@ -7479,6 +7485,20 @@
           dyn_cast<OMPArrayShapingExpr>(I->getAssociatedExpression());
       const auto *UO = dyn_cast<UnaryOperator>(I->getAssociatedExpression());
       const auto *BO = dyn_cast<BinaryOperator>(I->getAssociatedExpression());
+      auto TNext = Next;
+      bool IsVarDerefAssoWithArray = false;
+      if (UO && UO->getOpcode() == UO_Deref)
+        for (; TNext != CE; TNext = std::next(TNext))
+          if (isa<OMPArraySectionExpr>(TNext->getAssociatedExpression()) ||
+              isa<MemberExpr>(TNext->getAssociatedExpression()) ||
+              isa<OMPArrayShapingExpr>(TNext->getAssociatedExpression()) ||
+              isa<ArraySubscriptExpr>(TNext->getAssociatedExpression())) {
+            IsVarDerefAssoWithArray = true;
+            MapType = OMPC_MAP_alloc;
+            break;
+          }
+      if (!IsVarDerefAssoWithArray)
+        MapType = SaveMapType;
       bool IsPointer =
           OAShE ||
           (OASE && OMPArraySectionExpr::getBaseOriginalType(OASE)
@@ -7494,7 +7514,7 @@
         ++DimSize;
 
       if (Next == CE || IsMemberReference || IsNonDerefPointer ||
-          IsFinalArraySection) {
+          IsVarDerefAssoWithArray || IsFinalArraySection) {
         // If this is not the last component, we expect the pointer to be
         // associated with an array expression or member expression.
         assert((Next == CE ||
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to