ABataev added a comment.

I think we'd better to try to use attributes for declare target. I think it 
will be much easier and correct solution rather than adding a new declaration


================
Comment at: include/clang/AST/DeclBase.h:164
@@ -163,3 +163,3 @@
     /// has been declared outside any function.
-    IDNS_LocalExtern         = 0x0800
+    IDNS_LocalExtern         = 0x0800,
   };
----------------
I don't think this is required, revert it back please

================
Comment at: include/clang/AST/DeclOpenMP.h:18-21
@@ -17,4 +17,6 @@
 
-#include "clang/AST/DeclBase.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Type.h"
 #include "llvm/ADT/ArrayRef.h"
 
----------------
Again, I don't think this change is required. You don't use neither Exprs, nor 
Types in your changes below

================
Comment at: include/clang/AST/DeclOpenMP.h:98
@@ +97,3 @@
+///
+class OMPDeclareTargetDecl : public Decl, public DeclContext {
+       friend class ASTDeclReader;
----------------
I'm not sure that we need to add this kind of declaration. Most probably it is 
enough just to have an attribute

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7751
@@ -7745,1 +7750,3 @@
+  "declaration is not declared in any declare target region">,
+  InGroup<OpenMPClauses>;
 def err_omp_aligned_expected_array_or_ptr : Error<
----------------
I don't think that OpenMPClauses is suitable warning group for this warning. 
Maybe you need to add a new one, for example, target specific

================
Comment at: include/clang/Basic/OpenMPKinds.h:30
@@ -30,1 +29,3 @@
+  OMPD_unknown,
+  OMPD_empty
 };
----------------
OMPD_empty is not required, this must be deleted 

================
Comment at: lib/AST/Decl.cpp:21
@@ -20,2 +20,3 @@
 #include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/DeclTemplate.h"
----------------
Do you really need this header file here?

================
Comment at: lib/AST/DeclBase.cpp:653
@@ -652,2 +652,3 @@
     case OMPThreadPrivate:
+       case OMPDeclareTarget:
     case Empty:
----------------
The change is not properly formatted

================
Comment at: lib/AST/DeclOpenMP.cpp:10
@@ -9,3 +9,3 @@
 /// \file
-/// \brief This file implements OMPThreadPrivateDecl class.
+/// \brief This file implements OMPThreadPrivateDecl. OMPDeclareTarget class.
 ///
----------------
comma?

================
Comment at: lib/AST/DeclPrinter.cpp:95
@@ -94,2 +94,3 @@
     void VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D);
+       void VisitOMPDeclareTargetDecl(OMPDeclareTargetDecl *D);
 
----------------
Not formatted

================
Comment at: lib/AST/ItaniumMangle.cpp:23
@@ -22,2 +22,3 @@
 #include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/DeclTemplate.h"
----------------
I don't think this header file is required

================
Comment at: lib/AST/MicrosoftMangle.cpp:22
@@ -21,2 +21,3 @@
 #include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/DeclTemplate.h"
----------------
Again, Are you sure this is required?

================
Comment at: lib/CodeGen/CGDecl.cpp:1873-1874
@@ +1872,4 @@
+       // that is a valid region for a target
+       OpenMPSupport.startOpenMPRegion(false);
+       OpenMPSupport.setTargetDeclare(true);
+
----------------
There is no such object

================
Comment at: lib/CodeGen/CodeGenModule.h:1067-1102
@@ -1065,1 +1066,38 @@
 
+  class OpenMPSupportStackTy {
+         /// \brief A set of OpenMP threadprivate variables.
+         llvm::DenseMap<const Decl *, const Expr *> OpenMPThreadPrivate;
+         /// \brief A set of OpenMP private variables.
+         typedef llvm::DenseMap<const Decl *, llvm::Value *> OMPPrivateVarsTy;
+         struct OMPStackElemTy {
+                 OMPPrivateVarsTy PrivateVars;
+                 llvm::BasicBlock *IfEnd;
+                 Expr *IfClauseCondition;
+                 llvm::SmallVector<llvm::Value *, 16> offloadingMapArguments;
+                 llvm::Function *ReductionFunc;
+                 CodeGenModule &CGM;
+                 CodeGenFunction *RedCGF;
+                 llvm::SmallVector<llvm::Type *, 16> ReductionTypes;
+                 llvm::DenseMap<const VarDecl *, unsigned> ReductionMap;
+                 llvm::StructType *ReductionRec;
+                 llvm::Value *ReductionRecVar;
+                 llvm::Value *RedArg1;
+                 llvm::Value *RedArg2;
+                 llvm::Value *ReduceSwitch;
+                 llvm::BasicBlock *BB1;
+                 llvm::Instruction *BB1IP;
+                 llvm::BasicBlock *BB2;
+                 llvm::Instruction *BB2IP;
+                 llvm::Value *LockVar;
+                 llvm::BasicBlock *LastprivateBB;
+                 llvm::Instruction *LastprivateIP;
+                 llvm::BasicBlock *LastprivateEndBB;
+                 llvm::Value *LastIterVar;
+                 llvm::Value *TaskFlags;
+                 llvm::Value *PTaskTValue;
+                 llvm::Value *PTask;
+                 llvm::Value *UntiedPartIdAddr;
+                 unsigned     UntiedCounter;
+                 llvm::Value *UntiedSwitch;
+                 llvm::BasicBlock *UntiedEnd;
+                 CodeGenFunction *ParentCGF;
----------------
No, this must be deleted. We don't use it here

================
Comment at: lib/Parse/ParseDecl.cpp:3624
@@ -3623,3 +3623,3 @@
       // Silence possible warnings.
-      (void)Res;
+         (void)Res;
       continue;
----------------
Restore original line

================
Comment at: lib/Parse/ParseOpenMP.cpp:36-39
@@ +35,6 @@
+       OMPD_cancellation_point},                                       //0
+         {OMPD_unknown /*declare*/, OMPD_target /*target*/,
+          OMPD_declare_target },                                          //1
+         {OMPD_unknown /*end*/, OMPD_unknown /*declare*/,
+          OMPD_end_declare_target },                                      //2
+      {OMPD_target, OMPD_unknown /*data*/, OMPD_target_data},          //3
----------------
Formatting broken

================
Comment at: lib/Parse/ParseOpenMP.cpp:58-63
@@ -54,1 +57,8 @@
+           !P.getPreprocessor().getSpelling(Tok).compare("cancellation") ||
+                 ((i == 1 || i == 3) &&
+                  !P.getPreprocessor().getSpelling(Tok).compare("declare")) ||
+                 ((i == 2) &&
+                  !P.getPreprocessor().getSpelling(Tok).compare("end")) ||
+                 ((i == 3) && 
+                  !P.getPreprocessor().getSpelling(Tok).compare("target"));
     } else {
----------------
Formatting

================
Comment at: lib/Parse/ParseOpenMP.cpp:80-83
@@ -69,3 +79,6 @@
              !P.getPreprocessor().getSpelling(Tok).compare("point")) ||
-            ((i == 1) && 
!P.getPreprocessor().getSpelling(Tok).compare("data"));
+                       ((i == 2) &&
+                        
!P.getPreprocessor().getSpelling(Tok).compare("declare")) ||
+                       ((i == 3) && 
+                        !P.getPreprocessor().getSpelling(Tok).compare("data"));
       } else {
----------------
Formatting

================
Comment at: lib/Parse/ParseOpenMP.cpp:209
@@ -142,2 +208,3 @@
   case OMPD_taskloop_simd:
+  default:
     Diag(Tok, diag::err_omp_unexpected_directive)
----------------
Do not add default:, coding standard recommends to not use it

================
Comment at: lib/Parse/ParseOpenMP.cpp:387-392
@@ -318,2 +386,8 @@
     break;
+  default:
+         Diag(Tok, diag::err_omp_unexpected_directive)
+                 << getOpenMPDirectiveName(DKind);
+         while (!SkipUntil(tok::annot_pragma_openmp_end))
+                 ;
+         break;
   }
----------------
Do not add it

================
Comment at: lib/Parse/ParseOpenMP.cpp:394
@@ -319,2 +393,3 @@
   }
+
   return Directive;
----------------
Restore original

================
Comment at: lib/Sema/SemaExpr.cpp:13566-13568
@@ -13565,1 +13565,5 @@
                                Decl *D, Expr *E, bool OdrUse) {
+  if (SemaRef.IsDeclContextInOpenMPTarget(SemaRef.CurContext)) {
+       SemaRef.CheckDeclIsAllowedInOpenMPTarget(E, D);
+  }
+               
----------------
Formatting. Remove braces

================
Comment at: lib/Sema/SemaOpenMP.cpp:1139-1140
@@ -1105,3 +1138,4 @@
   QualType ExprType = VD->getType().getNonReferenceType();
-  ExprResult DE = buildDeclRefExpr(*this, VD, ExprType, Id.getLoc());
+  //ExprResult DE = buildDeclRefExpr(*this, VD, ExprType, Id.getLoc());
+  ExprResult DE = BuildDeclRefExpr(VD, ExprType, VK_LValue, Id.getLoc());
   return DE;
----------------
I don't understand why you changed this.

================
Comment at: lib/Sema/SemaOpenMP.cpp:1414-1415
@@ +1413,4 @@
+
+static bool IsCXXRecordForMappable(Sema &SemaRef, SourceLocation Loc,
+       DSAStackTy *Stack, CXXRecordDecl *RD);
+
----------------
Why you need this declaration if earlier you have a definition?

================
Comment at: lib/Serialization/ASTReaderDecl.cpp:353
@@ -352,2 +352,3 @@
     void VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D);
+       void VisitOMPDeclareTargetDecl(OMPDeclareTargetDecl *D);
 
----------------
Formatting

================
Comment at: lib/Serialization/ASTReaderDecl.cpp:2411
@@ -2406,1 +2410,3 @@
+      isa<OMPThreadPrivateDecl>(D) ||
+         isa<OMPDeclareTargetDecl>(D) )
     return true;
----------------
Formatting

================
Comment at: lib/Serialization/ASTReaderDecl.cpp:3307-3309
@@ -3300,2 +3306,5 @@
     break;
+  case DECL_OMP_DECLARETARGET:
+       D = OMPDeclareTargetDecl::CreateDeserialized(Context, ID);
+       break;
   case DECL_EMPTY:
----------------
Formatting

================
Comment at: lib/Serialization/ASTWriterDecl.cpp:134
@@ -133,2 +133,3 @@
     void VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D);
+       void VisitOMPDeclareTargetDecl(OMPDeclareTargetDecl *D);
 
----------------
Formatting


Repository:
  rL LLVM

http://reviews.llvm.org/D15321



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to