Support for OpenMP 5.0 sec 2.12.7 - Declare Target initializer expressions

[RitanyaB]

Summary

Initial support for OpenMP 5.0 declare target "as if" behavior for "initializer expressions".
If a variable with static storage duration or a function (except lambda for C++) is referenced in the initializer expression list of a variable with static storage duration that appears as a list item in a to clause on a declare target directive then the name of the referenced variable or function is treated as if it had appeared in a to clause on a declare target directive.

[ABataev]

clang/lib/Parse/ParseDecl.cpp

2093–2115

It has nothing to do with parsing, sema analysis. Make it part of Sema::checkDeclIsAllowedInOpenMPTarget

2107

Try to avoid recursion here

[RitanyaB]

clang/lib/Parse/ParseDecl.cpp

2107

Consider the following example-

static int var1 = 10; 
static int *var2 = &var1;
#pragma omp declare target
int **ptr1=&var2;
#pragma omp end declare target

In this case, by default ptr1 gets OMPDeclareTargetDeclAttr and the function mentioned above (ParseImplicitDeclareTargetAttr) would add OMPDeclareTargetDeclAttr to var2. But, in order to add OMPDeclareTargetDeclAttr to var1, recursion would be required.

[ABataev]

clang/lib/Parse/ParseDecl.cpp

2107

I do not ask to remove it, but use extra memory (SamllVector) to save the decls, requiring analysis, and iterate through this vector.

[RitanyaB]

clang/lib/Parse/ParseDecl.cpp

2093–2115

The Declaration in Sema::checkDeclIsAllowedInOpenMPTarget is incomplete.

VarDecl 0x1582b278 <test3.c:6:1, col:7> col:7 ptr1 'int **'
`-OMPDeclareTargetDeclAttr 0x1582b2e0 <line:5:21> Implicit MT_To DT_Any 1
  `-<<<NULL>>>

At this point, I do not have access to the initializer expression. Any suggestions?

[ABataev]

clang/lib/Parse/ParseDecl.cpp

2093–2115

The try to do it ActOnOpenMPDeclareTargetName

[RitanyaB]

clang/lib/Parse/ParseDecl.cpp

2093–2115

In the absence of a declare target clause, this function is not invoked (in the examples used above, the function is never called). Do you have any suggestions?

[ABataev]

clang/lib/Parse/ParseDecl.cpp

2093–2115

For such decls use checkDeclIsAllowedInOpenMPTarget

[RitanyaB]

clang/lib/Parse/ParseDecl.cpp

2093–2115

As mentioned in my previous comment, the Decl in checkDeclIsAllowedInOpenMPTarget is not complete and I don't have access to the initializer expression hence I am unable to move my functionality into checkDeclIsAllowedInOpenMPTarget.
After ParseDeclarationAfterDeclaratorAndAttributes returns, the Decl is complete and I can process it accordingly.

[ABataev]

clang/lib/Parse/ParseDecl.cpp

2093–2115

For decls, explicitly marked as DeclareTarget, use The try to do it ActOnOpenMPDeclareTargetName. For decls, referenced in the initializers, use checkDeclIsAllowedInOpenMPTarget.

[RitanyaB]

Adding the support in SemaOpenMP.cpp.

[ABataev]

clang/lib/Sema/SemaDecl.cpp

14467–14468 ↗

(On Diff #516733)

if (auto *VD = dyn_cast_or_null<VarDecl>(D); LangOpts.OpenMP && VD && VD->hasAttr<OMPDeclareTargetDeclAttr>() && VD->hasGlobalStorage())

clang/lib/Sema/SemaOpenMP.cpp

23091 ↗

(On Diff #516733)

static void addOMP...

23092 ↗

(On Diff #516733)

SmallVectorImpl<Decl *> &DeclVector

23093–23100 ↗

(On Diff #516733)

if (!DeclRef)

return;

if (auto *VD = dyn_cast_or_null<VarDecl>(DeclRef->getDecl()) {

DeclVar->addAttr(TargetDecl->getAttr<OMPDeclareTargetDeclAttr>());
DeclVector.push_back(DeclVar);

}

23103–23114 ↗

(On Diff #516733)

Use StmtVisitor to implement this correctly.

23120 ↗

(On Diff #516733)

100 is to much, use either default value SmallVector<Decl *> DeclVector; or smaller value (4 should be enough)

23122 ↗

(On Diff #516733)

while (!DeclVector.empty())

23123–23124 ↗

(On Diff #516733)

Decl *D = DeclVector.pop_back_val();

23125–23126 ↗

(On Diff #516733)

Use dyn_cast instead of isa/cast

[ABataev]

clang/lib/Sema/SemaDecl.cpp

14473 ↗

(On Diff #520729)

В is already checked that is not nullptr, so just a dyn_cast should be enough

clang/lib/Sema/SemaOpenMP.cpp

23094–23095 ↗

(On Diff #520729)

Why public?

23098 ↗

(On Diff #520729)

const auto *VD

23099 ↗

(On Diff #520729)

Use VD instead of this and drop const

23100–23101 ↗

(On Diff #520729)

Drop this and extra parens

23107–23110 ↗

(On Diff #520729)

Just Visit(*it)?

[RitanyaB]

clang/lib/Sema/SemaOpenMP.cpp

23094–23095 ↗

(On Diff #520729)

As the data members are accessed from outside the class (in ActOnOpenMPImplicitDeclareTarget function), I have made them public.

23107–23110 ↗

(On Diff #520729)

Or I can call VisitDeclRefExpr directly. Would that be more suitable?

[ABataev]

clang/lib/Sema/SemaOpenMP.cpp

23094–23095 ↗

(On Diff #520729)

If you need them, implement getters for them and make them private.

23106–23107 ↗

(On Diff #521014)

Why just a regular Visit does not work here? Plus, isa + dyn_cast is weird.

[ABataev]

clang/include/clang/Sema/Sema.h

11329 ↗

(On Diff #521902)

Do I understand correctly, that the variable itself is marked explicitly, but the initializer shall be marked implicitly? If so, please fix the function name

clang/lib/Sema/SemaOpenMP.cpp

23097–23098 ↗

(On Diff #521902)

Remove this->, where not required.

23099 ↗

(On Diff #521902)

1. Wrong function name format.
2. Do you really need to make it public? Hide as much as possible.

23102 ↗

(On Diff #521902)

Same, as for Push

23126–23140 ↗

(On Diff #521902)

Better to hide this in GlobalDeclRefChecker

[ABataev]

clang/lib/Sema/SemaDecl.cpp

14475–14477 ↗

(On Diff #524290)

Remove braces

clang/lib/Sema/SemaOpenMP.cpp

23091 ↗

(On Diff #524290)

Please, add comments, describing the class and its functionality

23095 ↗

(On Diff #524290)

setTargetDecl

23096 ↗

(On Diff #524290)

1. checkDeclVector
2. const member function

23098 ↗

(On Diff #524290)

const member function

23104 ↗

(On Diff #524290)

Remove this->

23113 ↗

(On Diff #524290)

1. Better to make it static member function.
2. Follow the rules for function names.

[ABataev]

clang/lib/Sema/SemaOpenMP.cpp

23121–23122 ↗

(On Diff #525531)

for (const Expr *Child : Ex->children())

23138–23143 ↗

(On Diff #525531)

Shall just something like this work here:

if (Ex)
  Visit(Ex);

[ABataev]

clang/lib/Sema/SemaOpenMP.cpp

23136 ↗

(On Diff #526377)

Better to expand auto here

23137 ↗

(On Diff #526377)

Formatting?

[ABataev]

clang/lib/Sema/SemaOpenMP.cpp

23106 ↗

(On Diff #526665)

Use just a StmtVisitor, if you still dropping const modifier.

23114 ↗

(On Diff #526665)

remove const_cast

23131 ↗

(On Diff #526665)

You don't need to use dyn_cast here, use just cast

[ABataev]

clang/lib/Sema/SemaOpenMP.cpp

23127 ↗

(On Diff #526694)

Fix the strange symbol in the comment

23129 ↗

(On Diff #526694)

Function names should be verb phrases (as they represent actions), and command-like function should be imperative. (https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly)

[ABataev]

LG

[DavidSpickett]

clang/lib/Sema/SemaOpenMP.cpp

23148 ↗

(On Diff #527351)

FYI I fixed an unused var warning here: https://github.com/llvm/llvm-project/commit/f3ca99a87c566fb5e910071f4cbb474ddb4e7f37

Potentially you could pass the result of the cast into declareTargetInitializer but I don't know enough about the code to change that too.

clang/lib/Parse/ParseDecl.cpp

while (true) {
     default:
       break;
     }
 
     ConsumeAnyToken();
   }
 }
 
+/// Adding OMPDeclareTargetDeclAttr to variables with static storage
+/// duration that is referenced in the initializer expression list of
+/// variables with static storage duration in declare target directive.
+void ParseImplicitDeclareTargetAttr(Decl *TargetDecl) {
+  if (TargetDecl && TargetDecl->hasAttr<OMPDeclareTargetDeclAttr>() &&
+      isa<VarDecl>(TargetDecl)) {
+    VarDecl *TargetVarDecl = cast<VarDecl>(TargetDecl);
+    Expr *Ex = TargetVarDecl->getInit()->IgnoreCasts();
+    const DeclRefExpr *DeclRef = nullptr;
+    if (Ex && isa<UnaryOperator>(Ex) && TargetVarDecl->hasGlobalStorage()) {
+      if (auto *unary = cast<UnaryOperator>(Ex)) {
+        if (unary->getOpcode() == UnaryOperator::Opcode::UO_AddrOf) {
+          for (auto child : unary->children()) {
+            if (isa<DeclRefExpr>(child)) {
+              DeclRef = dyn_cast<DeclRefExpr>(child);
+              Decl *DeclVar = (Decl *)DeclRef->getDecl();
+              DeclVar->addAttr(TargetDecl->getAttr<OMPDeclareTargetDeclAttr>());
+              ParseImplicitDeclareTargetAttr(DeclVar);
+            }
+          }
+        }
+      }
+    }
+  }
+  return;
+}
+
 /// ParseDeclGroup - Having concluded that this is either a function
 /// definition or a group of object declarations, actually parse the
 /// result.
 Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
                                               DeclaratorContext Context,
                                               ParsedAttributes &Attrs,
                                               SourceLocation *DeclEnd,
                                               ForRangeInit *FRI) {
if (FRI && (Tok.is(tok::colon) || isTokIdentifier_in())) {
     Actions.FinalizeDeclaration(ThisDecl);
     D.complete(ThisDecl);
     return Actions.FinalizeDeclaratorGroup(getCurScope(), DS, ThisDecl);
   }
 
   SmallVector<Decl *, 8> DeclsInGroup;
   Decl *FirstDecl = ParseDeclarationAfterDeclaratorAndAttributes(
       D, ParsedTemplateInfo(), FRI);
+  if (FirstDecl && FirstDecl->hasAttr<OMPDeclareTargetDeclAttr>() &&
+      isa<VarDecl>(FirstDecl)) {
+    VarDecl *VarFirstDecl = cast<VarDecl>(FirstDecl);
+    if (VarFirstDecl->hasGlobalStorage())
+      ParseImplicitDeclareTargetAttr(FirstDecl);
+  }
   if (LateParsedAttrs.size() > 0)
     ParseLexedAttributeList(LateParsedAttrs, FirstDecl, true, false);
   D.complete(FirstDecl);
   if (FirstDecl)
     DeclsInGroup.push_back(FirstDecl);
 
   bool ExpectSemi = Context != DeclaratorContext::ForInit;
 

clang/test/OpenMP/declare_target_messages.cpp

 void any7() {device();} // host5-error {{function with 'device_type(nohost)' is not available on host}}
 void any8() {any2();}
 
 int MultiDevTy;
 #pragma omp declare target to(MultiDevTy) device_type(any)    // omp45-error {{unexpected 'device_type' clause, only 'to' or 'link' clauses expected}} omp52-error {{unexpected 'to' clause, use 'enter' instead}} omp52-error {{expected at least one 'enter', 'link' or 'indirect' clause}}
 #pragma omp declare target to(MultiDevTy) device_type(host)   // omp45-error {{unexpected 'device_type' clause, only 'to' or 'link' clauses expected}} omp5-error {{'device_type(host)' does not match previously specified 'device_type(any)' for the same declaration}} omp51-error {{'device_type(host)' does not match previously specified 'device_type(any)' for the same declaration}} omp52-error {{unexpected 'to' clause, use 'enter' instead}} omp52-error {{expected at least one 'enter', 'link' or 'indirect' clause}}
 #pragma omp declare target to(MultiDevTy) device_type(nohost) // omp45-error {{unexpected 'device_type' clause, only 'to' or 'link' clauses expected}} omp5-error {{'device_type(nohost)' does not match previously specified 'device_type(any)' for the same declaration}} // omp51-error {{'device_type(nohost)' does not match previously specified 'device_type(any)' for the same declaration}} omp52-error {{unexpected 'to' clause, use 'enter' instead}} omp52-error {{expected at least one 'enter', 'link' or 'indirect' clause}}
 
+static int var = 1;  //expected-warning {{declaration is not declared in any declare target region}}
+
+static int var1 = 10; 
+static int *var2 = &var1;
+static int **ptr1 = &var2;  //expected-warning {{declaration is not declared in any declare target region}}
+
+int arr[2] = {1,2};
+int (*arrptr)[2] = &arr;  //expected-warning {{declaration is not declared in any declare target region}}
+
+class declare{
+  public: int x;
+    void print();
+};
+declare obj1;
+declare *obj2 = &obj1;  //expected-warning {{declaration is not declared in any declare target region}}
+
+struct target{
+  int x;
+  void print();
+};
+static target S;  //expected-warning {{declaration is not declared in any declare target region}}
+
+#pragma omp declare target
+int *ptr = &var;  //expected-note {{used here}}
+int ***ptr2 = &ptr1;  //expected-note {{used here}}
+int (**ptr3)[2] = &arrptr;  //expected-note {{used here}}
+declare **obj3 = &obj2;  //expected-note {{used here}}
+target *S1 = &S; //expected-note {{used here}}
+#pragma omp end declare target
+
 #if TESTENDINC
 #include "unterminated_declare_target_include.h"
 #elif TESTEND
 // expected-warning@+1 {{expected '#pragma omp end declare target' at end of file to match '#pragma omp declare target'}}
 #pragma omp declare target
 #else
 // expected-warning@+1 {{expected '#pragma omp end declare target' at end of file to match '#pragma omp begin declare target'}}
 #pragma omp begin declare target
 #endif

clang/test/OpenMP/declare_target_variables_ast_print.cpp

+// RUN: %clang_cc1 -w -verify -fopenmp -I %S/Inputs -ast-print %s | FileCheck %s --check-prefix=CHECK
+// expected-no-diagnostics
+
+static int var = 1;
+
+static int var1 = 10;
+static int *var2 = &var1;
+static int **ptr1 = &var2;
+
+int arr[2] = {1,2};
+int (*arrptr)[2] = &arr;
+
+class declare{
+  public: int x;
+          void print();
+};
+declare obj1;
+declare *obj2 = &obj1;
+
+struct target{
+  int x;
+  void print();
+};
+static target S;
+
+#pragma omp declare target
+int *ptr = &var;
+int ***ptr2 = &ptr1;
+int (**ptr3)[2] = &arrptr;
+declare **obj3 = &obj2;
+target *S1 = &S;
+#pragma omp end declare target
+// CHECK: #pragma omp declare target
+// CHECK-NEXT: static int var = 1;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static int var1 = 10;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static int *var2 = &var1;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static int **ptr1 = &var2;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int arr[2] = {1, 2};
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int (*arrptr)[2] = &arr;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: class declare {
+// CHECK-NEXT: public: 
+// CHECK-NEXT:  int x;
+// CHECK-NEXT:  void print();
+// CHECK-NEXT: };
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: declare obj1;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: declare *obj2 = &obj1;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: struct target {
+// CHECK-NEXT:  int x;
+// CHECK-NEXT:  void print();
+// CHECK-NEXT: };
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: static target S;
+// CHECK-NEXT: #pragma omp end declare target
+
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int *ptr = &var;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int ***ptr2 = &ptr1;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: int (**ptr3)[2] = &arrptr;
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: declare **obj3 = &obj2; 
+// CHECK-NEXT: #pragma omp end declare target
+// CHECK-NEXT: #pragma omp declare target
+// CHECK-NEXT: target *S1 = &S;
+// CHECK-NEXT: #pragma omp end declare target

clang/test/OpenMP/nvptx_target_exceptions_messages.cpp

 
 int foobar3();
 int (*C)() = &foobar3; // expected-warning {{declaration is not declared in any declare target region}}
                        // host-warning@-1 {{declaration is not declared in any declare target region}}
 #pragma omp declare target
 int (*D)() = C; // expected-note {{used here}}
                 // host-note@-1 {{used here}}
 #pragma omp end declare target
-int foobar3() { throw 1; }
+int foobar3() { throw 1; } // expected-error {{cannot use 'throw' with exceptions disabled}}
 
 // Check no infinite recursion in deferred diagnostic emitter.
 long E = (long)&E;
 
 #endif // HEADER