[PATCH] D146418: Support for OpenMP 5.0 sec 2.12.7 - Declare Target initializer expressions

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:14467-14468
+      // directive and has static storage duration.
+      if (D && D->hasAttr<OMPDeclareTargetDeclAttr>() && isa<VarDecl>(D)) {
+        if ((cast<VarDecl>(D))->hasGlobalStorage())
+          ActOnOpenMPImplicitDeclareTarget(D);
----------------
if (auto *VD = dyn_cast_or_null<VarDecl>(D); LangOpts.OpenMP && VD && VD->hasAttr<OMPDeclareTargetDeclAttr>() && VD->hasGlobalStorage())


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23091
+/// Adds OMPDeclareTargetDeclAttr to the referenced variables.
+void *AddOMPDeclareTargetDeclAttr(const DeclRefExpr *DeclRef, Decl *TargetDecl, 
+                                  SmallVector<Decl *, 100> &DeclVector) {
----------------
static void addOMP...


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23092
+void *AddOMPDeclareTargetDeclAttr(const DeclRefExpr *DeclRef, Decl *TargetDecl, 
+                                  SmallVector<Decl *, 100> &DeclVector) {
+  Decl *DeclVar = nullptr;
----------------
SmallVectorImpl<Decl *> &DeclVector


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23093-23100
+  Decl *DeclVar = nullptr;
+  if (DeclRef) {
+    DeclVar = (Decl *)DeclRef->getDecl();
+    if (isa<VarDecl>(DeclVar)) { 
+      DeclVar->addAttr(TargetDecl->getAttr<OMPDeclareTargetDeclAttr>());
+      DeclVector.push_back(DeclVar);
+    }
----------------
if (!DeclRef)
  return;
if (auto *VD = dyn_cast_or_null<VarDecl>(DeclRef->getDecl()) {
  DeclVar->addAttr(TargetDecl->getAttr<OMPDeclareTargetDeclAttr>());
  DeclVector.push_back(DeclVar);
}


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23103-23114
+void RecursiveExprReader(Expr *Ex, Decl *TargetDecl, 
+                         SmallVector<Decl *, 100> &DeclVector) {
+  for (Expr::child_iterator it = Ex->child_begin(); it != Ex->child_end(); 
+       ++it) {
+    if (const DeclRefExpr *refExpr = dyn_cast<DeclRefExpr>(*it)) {
+      AddOMPDeclareTargetDeclAttr(refExpr, TargetDecl, DeclVector);
+    } else if (isa<Expr>(*it))
----------------
Use StmtVisitor to implement this correctly.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23120
+void Sema::ActOnOpenMPImplicitDeclareTarget(Decl *TargetDecl) {
+  SmallVector<Decl *, 100> DeclVector;
+  DeclVector.push_back(TargetDecl);
----------------
100 is to much, use either default value `SmallVector<Decl *> DeclVector;` or smaller value (4 should be enough)


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23122
+  DeclVector.push_back(TargetDecl);
+  while (DeclVector.size() != 0) {
+    Decl *D = DeclVector[DeclVector.size() - 1];
----------------
while (!DeclVector.empty())


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23123-23124
+  while (DeclVector.size() != 0) {
+    Decl *D = DeclVector[DeclVector.size() - 1];
+    DeclVector.pop_back();
+    if (isa<VarDecl>(D) && D->hasAttr<OMPDeclareTargetDeclAttr>()) {
----------------
Decl *D = DeclVector.pop_back_val();


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23125-23126
+    DeclVector.pop_back();
+    if (isa<VarDecl>(D) && D->hasAttr<OMPDeclareTargetDeclAttr>()) {
+      VarDecl *TargetVarDecl = cast<VarDecl>(D);
+      if (TargetVarDecl->hasInit() && TargetVarDecl->hasGlobalStorage()) {
----------------
Use dyn_cast instead of isa/cast


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146418/new/

https://reviews.llvm.org/D146418