[OpenMP] Fix incorrect diagnostics in map clause

[• davidsh]

Summary

Having the following code pattern will result in incorrect diagnostic
int main() {
int arr[10];
#pragma omp target data map(arr[:])
#pragma omp target map(arr)
{}
}
t.cpp:4:24: error: original storage of expression in data environment is shared but data environment do not fully contain mapped expression storage
#pragma omp target map(arr)

^~~

t.cpp:3:29: note: used here
#pragma omp target data map(arr[:])

^~~~~~

1 error generated.

[sfantao]

Hi David,

Thanks for the fix!

lib/Sema/SemaOpenMP.cpp

10651–10670

You have to document what you are trying to accomplish here, something like "Check if the extra components of the expressions in the enclosing data environment are redundant for the current base declaration. If they are, the maps completely overlap, which is legal."

You may add the snippet in that comment so that we can refer to that when trying to understand this code.

10666

I'd rather use a for loop, move the iterator increment there, and invert the sense of this last if statement to break the loop.

[• davidsh]

Address comments

[sfantao]

Thanks. You have to wait for Alexey to take a look at the patch too.

Thanks again,
Samuel

[ABataev]

lib/Sema/SemaOpenMP.cpp

10667–10668

Seems to me the code is not formatted properly.

[• davidsh]

Address formatting comment.

[ABataev]

LG

[• davidsh]

Fix a typo in code and address test case issues.

lib/Sema/SemaOpenMP.cpp

bool FoundError = DSAS->checkMappableExprComponentListsForDecl(
           // Do both expressions have the same kind?
           if (CI->getAssociatedExpression()->getStmtClass() !=
               SI->getAssociatedExpression()->getStmtClass())
             break;
 
           // Are we dealing with different variables/fields?
           if (CI->getAssociatedDeclaration() != SI->getAssociatedDeclaration())
             break;
         }
+        // Check if the extra components of the expressions in the enclosing
+        // data environment are redundant for the current base declaration.
+        // If they are, the maps completely overlap, which is legal.
+        for (; SI != SE; ++SI) {
+          QualType Type;
+          if (auto *ASE =
+              dyn_cast<ArraySubscriptExpr>(SI->getAssociatedExpression())) {
+            Type = ASE->getBase()->IgnoreParenImpCasts()->getType();
+          } else if (auto *OASE =
+              dyn_cast<OMPArraySectionExpr>(SI->getAssociatedExpression())) {
+            auto *E = OASE->getBase()->IgnoreParenImpCasts();
+            Type =
+                OMPArraySectionExpr::getBaseOriginalType(E).getCanonicalType();
+          }
+          if (Type.isNull() || !Type->isAnyPointerType() ||
+              CheckArrayExpressionDoesNotReferToWholeSize(
+                  SemaRef,SI->getAssociatedExpression(),Type))
+            break;
+        }
 
         // OpenMP 4.5 [2.15.5.1, map Clause, Restrictions, p.4]
         //  List items of map clauses in the same construct must not share
         //  original storage.
         //
         // If the expressions are exactly the same or one is a subset of the
         // other, it means they are sharing storage.
         if (CI == CE && SI == SE) {

test/OpenMP/target_map_messages.cpp

void SAclient(int arg) {
   #pragma omp target data map(to: t.C, t.D)
   {
   #pragma omp target data map(to: t.C)
   {
     #pragma omp target map(t.D)
     {}
   }
   }
+  #pragma omp target data map(marr[:][:][:])
+  {
+    #pragma omp target data map(marr)
+    {}
+  }
 
   #pragma omp target data map(to: t)
   {
   #pragma omp target data map(to: t.C)
   {
     #pragma omp target map(t.D)
     {}
   }

 #pragma omp target data map(da)
 #pragma omp target map(da[:4])
   foo();
 #pragma omp target data map(k, j, l) // expected-note {{used here}}
 #pragma omp target data map(k[:4]) // expected-error {{pointer cannot be mapped along with a section derived from itself}}
 #pragma omp target data map(j)
 #pragma omp target map(l) map(l[:5]) // expected-error {{variable already marked as mapped in current construct}} expected-note {{used here}}
   foo();
-#pragma omp target data map(k[:4], j, l[:5]) // expected-note 2 {{used here}}
+#pragma omp target data map(k[:4], j, l[:5]) // expected-note {{used here}}
 #pragma omp target data map(k) // expected-error {{pointer cannot be mapped along with a section derived from itself}}
 #pragma omp target data map(j)
-#pragma omp target map(l) // expected-error {{original storage of expression in data environment is shared but data environment do not fully contain mapped expression storage}}
+#pragma omp target map(l)
   foo();
 
 #pragma omp target data map(always, tofrom: x)
 #pragma omp target data map(always: x) // expected-error {{missing map type}}
 #pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always'}} expected-error {{incorrect map type, expected one of 'to', 'from', 'tofrom', 'alloc', 'release', or 'delete'}}
 #pragma omp target data map(always, tofrom: always, tofrom, x)
 #pragma omp target map(tofrom j) // expected-error {{expected ',' or ')' in 'map' clause}}
   foo();
 #pragma omp target private(j) map(j) // expected-error {{private variable cannot be in a map clause in '#pragma omp target' directive}}  expected-note {{defined as private}}
   {}
 #pragma omp target firstprivate(j) map(j)  // expected-error {{firstprivate variable cannot be in a map clause in '#pragma omp target' directive}} expected-note {{defined as firstprivate}}
   {}
   return tmain<int, 3>(argc)+tmain<from, 4>(argc); // expected-note {{in instantiation of function template specialization 'tmain<int, 3>' requested here}} expected-note {{in instantiation of function template specialization 'tmain<int, 4>' requested here}}
 }
 #endif