Re: [PATCH] D15321: [OpenMP 4.0]Parsing and Sema support for 'omp declare target' directive (accelerator support)

2015-12-29 Thread Michael Wong via cfe-commits 
fraggamuffin marked 15 inline comments as done.
fraggamuffin added a comment.

Thanks for the pre-xmas review.



Comment at: include/clang/AST/DeclOpenMP.h:98
@@ +97,3 @@
+///
+class OMPDeclareTargetDecl : public Decl, public DeclContext {
+   friend class ASTDeclReader;

ABataev wrote:
> I'm not sure that we need to add this kind of declaration. Most probably it 
> is enough just to have an attribute
I added attributes in a few places according to what was done with 
threadprivate but I am not sure it is enough. 


Comment at: lib/Parse/ParseOpenMP.cpp:209
@@ -142,2 +208,3 @@
   case OMPD_taskloop_simd:
+  default:
 Diag(Tok, diag::err_omp_unexpected_directive)

ABataev wrote:
> fraggamuffin wrote:
> > ABataev wrote:
> > > Do not add default:, coding standard recommends to not use it
> > This is needed to generate these error messages in 
> > declare_target_messages.cpp
> > error: 'error' diagnostics expected but not seen:
> >   File 
> > C:\llvm151206\llvm\tools\clang\test\OpenMP\declare_target_messages.cpp Li
> > ne 3: unexpected OpenMP directive '#pragma omp end declare target'
> >   File 
> > C:\llvm151206\llvm\tools\clang\test\OpenMP\declare_target_messages.cpp Li
> > ne 63: unexpected OpenMP directive '#pragma omp end declare target'
> >   File 
> > C:\llvm151206\llvm\tools\clang\test\OpenMP\declare_target_messages.cpp Li
> > ne 72: unexpected OpenMP directive '#pragma omp end declare target'
> > 3 errors generated.
> Still 'default:' is a bad solution. You must explicitly take all possible 
> cases for the enumeric type
I agree and see your point. Thanks.  I fixed it by adding the search for end 
declare target.


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;
   }

ABataev wrote:
> fraggamuffin wrote:
> > ABataev wrote:
> > > Do not add it
> > This is needed to prevent infinite loop when there is an unexpected end of 
> > the pragma omp target.
> As I said before 'default:' is a bad solution. Add explicit cases for all 
> possible values
I agree and see your point. Thanks.  I fixed it .


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;

ABataev wrote:
> fraggamuffin wrote:
> > ABataev wrote:
> > > I don't understand why you changed this.
> > The buildDeclRefExpr interface does not enable us to generate these 
> > messages.
> > Someone changed it to buildDeclRefExpr(...) which has a different interface
> > In the github repository it also uses 
> > ExprResult DE = BuildDeclRefExpr(VD, ExprType, VK_LValue, Id.getLoc());
> > 
> > With the buildDeclRefExpr interface, it misses generation of these messages
> > error: 'warning' diagnostics expected but not seen:
> >   File 
> > C:\llvm151206\llvm\tools\clang\test\OpenMP\declare_target_messages.cpp Li
> > ne 5: declaration is not declared in any declare target region
> > error: 'note' diagnostics expected but not seen:
> >   File 
> > C:\llvm151206\llvm\tools\clang\test\OpenMP\declare_target_messages.cpp Li
> > ne 21: used here
> > 2 errors generated.
> > 
> Github is not a reference version, it has many very bad and not working 
> solutions. You don't need take everything from there, some solutions are just 
> not compatible with trunk
OK, I looked at the uses of build.. and Build.. and see that build... is only 
used within SemaOpenMP.cpp, so I changed it back, and added my check inside 
build... for when it is in a target region. Thanks.


http://reviews.llvm.org/D15321



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

Re: [PATCH] D15321: [OpenMP 4.0]Parsing and Sema support for 'omp declare target' directive (accelerator support)

2015-12-09 Thread Michael Wong via cfe-commits 
fraggamuffin removed rL LLVM as the repository for this revision.
fraggamuffin updated this revision to Diff 42307.
fraggamuffin marked an inline comment as done.
fraggamuffin added a comment.

This is just an interim full diff file of changes accepted so far. I still need 
to address the attribute and Helper class comment. Thanks.


http://reviews.llvm.org/D15321

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclOpenMP.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclOpenMP.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/OpenMP/declare_target_ast_print.cpp
  clang/test/OpenMP/declare_target_messages.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -5228,6 +5228,7 @@
   case Decl::ClassScopeFunctionSpecialization:
   case Decl::Import:
   case Decl::OMPThreadPrivate:
+  case Decl::OMPDeclareTarget:
   case Decl::ObjCTypeParam:
   case Decl::BuiltinTemplate:
 return C;
Index: clang/test/OpenMP/declare_target_messages.cpp
===
--- clang/test/OpenMP/declare_target_messages.cpp
+++ clang/test/OpenMP/declare_target_messages.cpp
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify -fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - %s
+
+#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}}
+
+int a, b; // expected-warning 2 {{declaration is not declared in any declare target region}}
+__thread int t; // expected-error {{threadprivate variables cannot be used in target constructs}}
+#pragma omp declare target private(a) // expected-warning {{extra tokens at the end of '#pragma omp declare target' are ignored}}
+void f();
+#pragma omp end declare target shared(a) // expected-warning {{extra tokens at the end of '#pragma omp end declare target' are ignored}}
+void c(); // expected-warning {{declaration is not declared in any declare target region}}
+
+extern int b;
+
+struct NonT {
+  int a;
+};
+
+typedef int sint;
+
+#pragma omp declare target // expected-note {{to match this '#pragma omp declare target'}}
+#pragma omp threadprivate(a) // expected-error {{threadprivate variables cannot be used in target constructs}} expected-note {{used here}}
+extern int b;
+int g;
+
+struct T { // expected-note {{mappable type cannot be polymorphic}}
+  int a;
+  virtual int method();
+};
+
+class VC { // expected-note {{mappable type cannot be polymorphic}}
+  T member;
+  NonT member1;
+  public:
+virtual int method() { T a; return 0; } // expected-error {{type 'T' is not mappable to target}}
+};
+
+struct C {
+  NonT a;
+  sint b;
+  int method();
+  int method1();
+};
+
+int C::method1() {
+  return 0;
+}
+
+void foo() {
+  a = 0; // expected-note {{used here}}
+  b = 0; // expected-note {{used here}}
+  t = 1; // expected-note {{used here}}
+  C object;
+  VC object1; // expected-error {{type 'VC' is not mappable to target}}
+  g = object.method();
+  g += object.method1();
+  g += object1.method();
+  f();
+  c(); // expected-note {{used here}}
+}
+#pragma omp declare target // expected-error {{expected '#pragma omp end declare target'}}
+void foo1() {}
+#pragma omp end declare target
+#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}}
+
+int C::method() {
+  return 0;
+}
+
+struct S {
+#pragma omp declare target // expected-error {{directive must be at file or namespace scope}}
+  int v;
+#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}}
+};
+
+int main (int argc, char **argv) {
+#pragma omp declare target //

[PATCH] D15321: [OpenMP 4.0]Parsing and Sema support for 'omp declare target' directive (accelerator support)

2015-12-07 Thread Michael Wong via cfe-commits 
fraggamuffin created this revision.
fraggamuffin added a reviewer: cfe-commits.
fraggamuffin set the repository for this revision to rL LLVM.

Add parsing, sema analysis for 'declare target' construct for OpenMP 4.0.
Summary
The declare target directive specifies that variables, functions (C, C++ and
Fortran), and subroutines (Fortran) are mapped to a device. The declare target
directive is a declarative directive.

The syntax of the declare target directive is as follows:
C/C++
Fortran
The syntax of the declare target directive is as follows:

For variables, functions and subroutines:


```
#pragma omp declare target new-line
declarations-definition-seq
#pragma omp end declare target new-line
```

Codegen will be done in a separate delivery. This is based on the clang-omp 
github branch adapted for 3.8.
All unit tests passes (note that message test needed to add -fnoopenmp-use-tls 
to pass specifically the threadprivate part because use-tls is now set to 
default). All regression passes.
OpenMP 4.5 changes will be delivered separately.


Repository:
  rL LLVM

http://reviews.llvm.org/D15321

Files:
  include/clang/AST/DeclBase.h
  include/clang/AST/DeclOpenMP.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/DeclNodes.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Basic/OpenMPKinds.h
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/Decl.cpp
  lib/AST/DeclBase.cpp
  lib/AST/DeclOpenMP.cpp
  lib/AST/DeclPrinter.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Parse/Parser.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/OpenMP/declare_target_ast_print.cpp
  test/OpenMP/declare_target_messages.cpp
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -5228,6 +5228,7 @@
   case Decl::ClassScopeFunctionSpecialization:
   case Decl::Import:
   case Decl::OMPThreadPrivate:
+  case Decl::OMPDeclareTarget:
   case Decl::ObjCTypeParam:
   case Decl::BuiltinTemplate:
 return C;
Index: test/OpenMP/declare_target_messages.cpp
===
--- test/OpenMP/declare_target_messages.cpp
+++ test/OpenMP/declare_target_messages.cpp
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify -fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - %s
+
+#pragma omp end declare target // expected-error {{unexpected OpenMP directive '#pragma omp end declare target'}}
+
+int a, b; // expected-warning 2 {{declaration is not declared in any declare target region}}
+__thread int t; // expected-error {{threadprivate variables cannot be used in target constructs}}
+#pragma omp declare target private(a) // expected-warning {{extra tokens at the end of '#pragma omp declare target' are ignored}}
+void f();
+#pragma omp end declare target shared(a) // expected-warning {{extra tokens at the end of '#pragma omp end declare target' are ignored}}
+void c(); // expected-warning {{declaration is not declared in any declare target region}}
+
+extern int b;
+
+struct NonT {
+  int a;
+};
+
+typedef int sint;
+
+#pragma omp declare target // expected-note {{to match this '#pragma omp declare target'}}
+#pragma omp threadprivate(a) // expected-error {{threadprivate variables cannot be used in target constructs}} expected-note {{used here}}
+extern int b;
+int g;
+
+struct T { // expected-note {{mappable type cannot be polymorphic}}
+  int a;
+  virtual int method();
+};
+
+class VC { // expected-note {{mappable type cannot be polymorphic}}
+  T member;
+  NonT member1;
+  public:
+virtual int method() { T a; return 0; } // expected-error {{type 'T' is not mappable to target}}
+};
+
+struct C {
+  NonT a;
+  sint b;
+  int method();
+  int method1();
+};
+
+int C::method1() {
+  return 0;
+}
+
+void foo() {
+  a = 0; // expected-note {{used here}}
+  b = 0; // expected-note {{used here}}
+  t = 1; // expected-note {{used here}}
+  C object;
+  VC object1; // expected-error {{type 'VC' is not mappable to target}}
+  g = object.method();
+  g += object.method1();
+  g += object1.method();
+  f();
+  c(); // expected-note {{used here}}
+}
+#pragma omp declare target // expected-error {{expected '#pragma omp end declare target'}}
+void foo1() {}
+#pragma omp end declare target

Re: [PATCH] D11182: [OPENMP 4.0] Initial support for 'omp declare reduction' construct.

2015-08-24 Thread Michael Wong via cfe-commits 
fraggamuffin added a comment.

Got it. Test results look good with this patch.

  Expected Passes: 7011
  Expected Failures  : 21
  Unsupported Tests  : 89
  Unexpected Failures: 47


http://reviews.llvm.org/D11182



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

Re: [PATCH] D11182: [OPENMP 4.0] Initial support for 'omp declare reduction' construct.

2015-08-20 Thread Michael Wong via cfe-commits 
fraggamuffin added a comment.





Comment at: lib/Parse/ParseDeclCXX.cpp:3011
@@ -3010,3 +3010,3 @@
   if (Tok.is(tok::annot_pragma_openmp)) {
-ParseOpenMPDeclarativeDirective();
 continue;

While testing this patch with the latest trunk for my work on declare target, I 
kept getting a build error in ParseDeclCXX.cpp as there seems to be no CurAS in 
scope, this may be because of recent changes. But this line:
  if (Tok.is(tok::annot_pragma_openmp))
return ParseOpenMPDeclarativeDirective(CurAS);
Seems to work better as
  if (Tok.is(tok::annot_pragma_openmp))
return ParseOpenMPDeclarativeDirective(AS);


http://reviews.llvm.org/D11182



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

r244569 - This patch fixes the assert in emitting captured code in the target data construct.

2015-08-10 Thread Michael Wong via cfe-commits 
Author: fraggamuffin
Date: Mon Aug 10 23:52:01 2015
New Revision: 244569

URL: http://llvm.org/viewvc/llvm-project?rev=244569view=rev
Log:
This patch fixes the assert in emitting captured code in the target data 
construct.
This is on behalf of Kelvin Li.
http://reviews.llvm.org/D11475

Modified:
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=244569r1=244568r2=244569view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Mon Aug 10 23:52:01 2015
@@ -2156,5 +2156,7 @@ void CodeGenFunction::EmitOMPTargetDataD
 
   // emit the code inside the construct for now
   auto CS = castCapturedStmt(S.getAssociatedStmt());
-  EmitStmt(CS-getCapturedStmt());
+  CGM.getOpenMPRuntime().emitInlinedDirective(
+  *this, OMPD_target_data,
+  [CS](CodeGenFunction CGF) { CGF.EmitStmt(CS-getCapturedStmt()); });
 }


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

Re: [PATCH] D11475: [OPENMP] fix the assert in emitting captured code inside the target data construct

2015-08-10 Thread Michael Wong via cfe-commits 
fraggamuffin added a comment.

This patch has landed.
Commit
C:\llvmtrunk\tools\clang\lib\CodeGen\CGStmtOpenMP.cpp
C:\llvmtrunk\tools\clang\lib\CodeGen\CGStmtOpenMP.cpp
At revision: 244569


http://reviews.llvm.org/D11475



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