[OpenMP] Fix assert fail after target implicit map checks

[jdenny]

Summary

An error like err_omp_union_type_not_allowed for an implicit mapping
on a target directive produced an assert failure.

[ABataev]

Fixed in a bit different way in r319774

[jdenny]

r319774 works for my current use cases. Thanks.

While we're on this topic, do you happen to know the rationale behind the OpenMP restriction for which err_omp_union_type_not_allowed diagnoses violations? I googled but couldn't find the rationale. If you would prefer that I ask this in a different forum, would you please suggest one? Thanks.

[ABataev]

In D40752#945234, @jdenny wrote:

r319774 works for my current use cases. Thanks.

While we're on this topic, do you happen to know the rationale behind the OpenMP restriction for which err_omp_union_type_not_allowed diagnoses violations? I googled but couldn't find the rationale. If you would prefer that I ask this in a different forum, would you please suggest one? Thanks.

You cannot map the member of the union, but you can map the whole union. Mapping of separate members is not allowed because you will definitely have troubles with overlapping memory for union members.

[ABataev]

In D40752#945234, @jdenny wrote:

r319774 works for my current use cases. Thanks.

While we're on this topic, do you happen to know the rationale behind the OpenMP restriction for which err_omp_union_type_not_allowed diagnoses violations? I googled but couldn't find the rationale. If you would prefer that I ask this in a different forum, would you please suggest one? Thanks.

Ahh, and in your example, there is a problem in the compiler. We should map the whole union implicitly. I'll fix this

[ABataev]

In D40752#945234, @jdenny wrote:

r319774 works for my current use cases. Thanks.

While we're on this topic, do you happen to know the rationale behind the OpenMP restriction for which err_omp_union_type_not_allowed diagnoses violations? I googled but couldn't find the rationale. If you would prefer that I ask this in a different forum, would you please suggest one? Thanks.

You can try it now, there should no more error messages

[jdenny]

In D40752#945255, @ABataev wrote:

In D40752#945234, @jdenny wrote:

r319774 works for my current use cases. Thanks.

While we're on this topic, do you happen to know the rationale behind the OpenMP restriction for which err_omp_union_type_not_allowed diagnoses violations? I googled but couldn't find the rationale. If you would prefer that I ask this in a different forum, would you please suggest one? Thanks.

You cannot map the member of the union, but you can map the whole union. Mapping of separate members is not allowed because you will definitely have troubles with overlapping memory for union members.

Thanks for that clarification. Is there any way to word the error message "mapped storage cannot be derived from a union" to make this point clearer? I'm thinking "an individual member of" instead of "derived from" would help. Does that work ok?

You can try it now, there should no more error messages

Yes. Thanks.

[ABataev]

In D40752#945571, @jdenny wrote:

In D40752#945255, @ABataev wrote:

In D40752#945234, @jdenny wrote:

r319774 works for my current use cases. Thanks.

While we're on this topic, do you happen to know the rationale behind the OpenMP restriction for which err_omp_union_type_not_allowed diagnoses violations? I googled but couldn't find the rationale. If you would prefer that I ask this in a different forum, would you please suggest one? Thanks.

You cannot map the member of the union, but you can map the whole union. Mapping of separate members is not allowed because you will definitely have troubles with overlapping memory for union members.

Thanks for that clarification. Is there any way to word the error message "mapped storage cannot be derived from a union" to make this point clearer? I'm thinking "an individual member of" instead of "derived from" would help. Does that work ok?

You can try it now, there should no more error messages

Yes. Thanks.

I agree, that error message does not sound quite good. Maybe, mapping of union members is not allowed?

[jdenny]

In D40752#945572, @ABataev wrote:

In D40752#945571, @jdenny wrote:

In D40752#945255, @ABataev wrote:

In D40752#945234, @jdenny wrote:

r319774 works for my current use cases. Thanks.

While we're on this topic, do you happen to know the rationale behind the OpenMP restriction for which err_omp_union_type_not_allowed diagnoses violations? I googled but couldn't find the rationale. If you would prefer that I ask this in a different forum, would you please suggest one? Thanks.

You cannot map the member of the union, but you can map the whole union. Mapping of separate members is not allowed because you will definitely have troubles with overlapping memory for union members.

Thanks for that clarification. Is there any way to word the error message "mapped storage cannot be derived from a union" to make this point clearer? I'm thinking "an individual member of" instead of "derived from" would help. Does that work ok?

You can try it now, there should no more error messages

Yes. Thanks.

I agree, that error message does not sound quite good. Maybe, mapping of union members is not allowed?

I like that.

[jdenny]

Alexey: I see that you committed the error message change, so I think this issue is done. Is Abandon Revision correct in this scenario? Sorry, I'm new here.

[ABataev]

In D40752#948399, @jdenny wrote:

Alexey: I see that you committed the error message change, so I think this issue is done. Is Abandon Revision correct in this scenario? Sorry, I'm new here.

Yes, it is correct

lib/Sema/SemaOpenMP.cpp

if (isa<CXXThisExpr>(E->getBase()->IgnoreParens())) {
       if (isOpenMPTaskingDirective(DKind) && DVar.CKind != OMPC_shared &&
           !Stack->isLoopControlVariable(FD).first)
         ImplicitFirstprivate.push_back(E);
       return;
     }
     if (isOpenMPTargetExecutionDirective(DKind) && !FD->isBitField()) {
       OMPClauseMappableExprCommon::MappableExprComponentList CurComponents;
       CheckMapClauseExpressionBase(SemaRef, E, CurComponents, OMPC_map);
+      if (CurComponents.empty())
+        return;
       auto *VD = cast<ValueDecl>(
           CurComponents.back().getAssociatedDeclaration()->getCanonicalDecl());
       if (!Stack->checkMappableExprComponentListsForDecl(
               VD, /*CurrentRegionOnly=*/true,
               [&CurComponents](
                   OMPClauseMappableExprCommon::MappableExprComponentListRef
                       StackComponents,
                   OpenMPClauseKind) {

test/OpenMP/target_map_messages.cpp

void SAclient(int arg) {
   #pragma omp target map(r.S.Ptr[4:5])
   {}
   #pragma omp target map(r.S.Ptr[:])  // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
   {}
   #pragma omp target map((p+1)->A)  // expected-error {{expected expression containing only member accesses and/or array sections based on named variables}}
   {}
   #pragma omp target map(u.B)  // expected-error {{mapped storage cannot be derived from a union}}
   {}
+  #pragma omp target
+  {
+    u.B = 0; // expected-error {{mapped storage cannot be derived from a union}}
+  }
 
   #pragma omp target data map(to: r.C) //expected-note {{used here}}
   {
     #pragma omp target map(r.D)  // expected-error {{original storage of expression in data environment is shared but data environment do not fully contain mapped expression storage}}
     {}
   }
 
   #pragma omp target data map(to: t.Ptr) //expected-note {{used here}}