subject:"\[PATCH\] D24615\: \[OpenMP\] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard"

[PATCH] D24615: [OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

2016-10-05 Thread Alexey Bataev via cfe-commits

ABataev added a comment.

In https://reviews.llvm.org/D24615#561532, @kkwli0 wrote:

> Should we issue a warning message in this case?


Agree


https://reviews.llvm.org/D24615



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

[PATCH] D24615: [OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

2016-10-04 Thread Kelvin Li via cfe-commits

kkwli0 added a comment.

Should we issue a warning message in this case?


https://reviews.llvm.org/D24615



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

[PATCH] D24615: [OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

2016-10-04 Thread David Sheinkman via cfe-commits

davidsh updated this revision to Diff 73520.
davidsh added a comment.

Adding a parameter to IgnoreContainers instead of copying the logic.


https://reviews.llvm.org/D24615

Files:
  include/clang/AST/Stmt.h
  lib/AST/Stmt.cpp
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/for_loop_messages.cpp


Index: test/OpenMP/for_loop_messages.cpp
===
--- test/OpenMP/for_loop_messages.cpp
+++ test/OpenMP/for_loop_messages.cpp
@@ -353,6 +353,14 @@
   }
 
 #pragma omp parallel
+// expected-error@+2 {{statement after '#pragma omp for' must be a for loop}}
+#pragma omp for
+  {
+  for (int i = 0; i < 16; ++i)
+;
+  }
+
+#pragma omp parallel
 // expected-note@+3 {{loop step is expected to be positive due to this 
condition}}
 // expected-error@+2 {{increment expression must cause 'i' to increase on each 
iteration of OpenMP for loop}}
 #pragma omp for
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -5133,7 +5133,8 @@
   llvm::MapVector Captures;
   SmallVector IterSpaces;
   IterSpaces.resize(NestedLoopCount);
-  Stmt *CurStmt = AStmt->IgnoreContainers(/* IgnoreCaptured */ true);
+  Stmt *CurStmt = AStmt->IgnoreContainers(/* IgnoreCaptured */ true, 
+  /* IgnoreCompound */ false);
   for (unsigned Cnt = 0; Cnt < NestedLoopCount; ++Cnt) {
 if (CheckOpenMPIterationSpace(DKind, CurStmt, SemaRef, DSA, Cnt,
   NestedLoopCount, CollapseLoopCountExpr,
Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -111,20 +111,24 @@
   return s;
 }
 
-/// \brief Skip no-op (attributed, compound) container stmts and skip captured
-/// stmt at the top, if \a IgnoreCaptured is true.
-Stmt *Stmt::IgnoreContainers(bool IgnoreCaptured) {
+/// \brief Skip no-op (attributed, compound if \a IgnoreCompound is true)
+/// container stmts and skip captured stmt at the top, if \a IgnoreCaptured
+/// is true.
+Stmt *Stmt::IgnoreContainers(bool IgnoreCaptured, bool IgnoreCompound) {
   Stmt *S = this;
   if (IgnoreCaptured)
 if (auto CapS = dyn_cast_or_null(S))
   S = CapS->getCapturedStmt();
   while (true) {
 if (auto AS = dyn_cast_or_null(S))
   S = AS->getSubStmt();
-else if (auto CS = dyn_cast_or_null(S)) {
-  if (CS->size() != 1)
-break;
-  S = CS->body_back();
+else if (IgnoreCompound) {
+  if (auto CS = dyn_cast_or_null(S)) {
+if (CS->size() != 1)
+  break;
+S = CS->body_back();
+  } else
+ break;
 } else
   break;
   }
Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -388,9 +388,11 @@
   /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
   Stmt *IgnoreImplicit();
 
-  /// \brief Skip no-op (attributed, compound) container stmts and skip 
captured
-  /// stmt at the top, if \a IgnoreCaptured is true.
-  Stmt *IgnoreContainers(bool IgnoreCaptured = false);
+  /// \brief Skip no-op (attributed, compound if \a IgnoreCompound is true)
+  /// container stmts and skip captured stmt at the top, if \a IgnoreCaptured
+  /// is true.
+  Stmt *IgnoreContainers(bool IgnoreCaptured = false, 
+ bool IgnoreCompound = true);
 
   const Stmt *stripLabelLikeStatements() const;
   Stmt *stripLabelLikeStatements() {


Index: test/OpenMP/for_loop_messages.cpp
===
--- test/OpenMP/for_loop_messages.cpp
+++ test/OpenMP/for_loop_messages.cpp
@@ -353,6 +353,14 @@
   }
 
 #pragma omp parallel
+// expected-error@+2 {{statement after '#pragma omp for' must be a for loop}}
+#pragma omp for
+  {
+  for (int i = 0; i < 16; ++i)
+;
+  }
+
+#pragma omp parallel
 // expected-note@+3 {{loop step is expected to be positive due to this condition}}
 // expected-error@+2 {{increment expression must cause 'i' to increase on each iteration of OpenMP for loop}}
 #pragma omp for
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -5133,7 +5133,8 @@
   llvm::MapVector Captures;
   SmallVector IterSpaces;
   IterSpaces.resize(NestedLoopCount);
-  Stmt *CurStmt = AStmt->IgnoreContainers(/* IgnoreCaptured */ true);
+  Stmt *CurStmt = AStmt->IgnoreContainers(/* IgnoreCaptured */ true, 
+  /* IgnoreCompound */ false);
   for (unsigned Cnt = 0; Cnt < NestedLoopCount; ++Cnt) {
 if (CheckOpenMPIterationSpace(DKind, CurStmt, SemaRef, DSA, Cnt,
   NestedLoopCount, CollapseLoopCountExpr,
Index: lib/AST/Stmt.cpp

Re: [PATCH] D24615: [OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

2016-09-16 Thread Alexey Bataev via cfe-commits

ABataev added inline comments.


Comment at: lib/Sema/SemaOpenMP.cpp:5137-5138
@@ -5137,1 +5136,4 @@
+  Stmt *CurStmt = AStmt;
+  if (auto CapS = dyn_cast_or_null(CurStmt))
+CurStmt = CapS->getCapturedStmt();
   for (unsigned Cnt = 0; Cnt < NestedLoopCount; ++Cnt) {

You also should skip all AttributedStmts.


https://reviews.llvm.org/D24615



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

[PATCH] D24615: [OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

2016-09-15 Thread David Sheinkman via cfe-commits

davidsh created this revision.
davidsh added reviewers: carlo.bertolli, arpith-jacob, kkwli0, sfantao, ABataev.
davidsh added a subscriber: cfe-commits.


#pragma omp for
{
for(...)
}

This is technically not allowed by the standard, gcc doesnt allow such code.


https://reviews.llvm.org/D24615

Files:
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/for_loop_messages.cpp

Index: test/OpenMP/for_loop_messages.cpp
===
--- test/OpenMP/for_loop_messages.cpp
+++ test/OpenMP/for_loop_messages.cpp
@@ -353,6 +353,14 @@
   }
 
 #pragma omp parallel
+// expected-error@+2 {{statement after '#pragma omp for' must be a for loop}}
+#pragma omp for
+  {
+  for (int i = 0; i < 16; ++i)
+;
+  }
+
+#pragma omp parallel
 // expected-note@+3 {{loop step is expected to be positive due to this 
condition}}
 // expected-error@+2 {{increment expression must cause 'i' to increase on each 
iteration of OpenMP for loop}}
 #pragma omp for
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -5133,7 +5133,9 @@
   llvm::MapVector Captures;
   SmallVector IterSpaces;
   IterSpaces.resize(NestedLoopCount);
-  Stmt *CurStmt = AStmt->IgnoreContainers(/* IgnoreCaptured */ true);
+  Stmt *CurStmt = AStmt;
+  if (auto CapS = dyn_cast_or_null(CurStmt))
+CurStmt = CapS->getCapturedStmt();
   for (unsigned Cnt = 0; Cnt < NestedLoopCount; ++Cnt) {
 if (CheckOpenMPIterationSpace(DKind, CurStmt, SemaRef, DSA, Cnt,
   NestedLoopCount, CollapseLoopCountExpr,


Index: test/OpenMP/for_loop_messages.cpp
===
--- test/OpenMP/for_loop_messages.cpp
+++ test/OpenMP/for_loop_messages.cpp
@@ -353,6 +353,14 @@
   }
 
 #pragma omp parallel
+// expected-error@+2 {{statement after '#pragma omp for' must be a for loop}}
+#pragma omp for
+  {
+  for (int i = 0; i < 16; ++i)
+;
+  }
+
+#pragma omp parallel
 // expected-note@+3 {{loop step is expected to be positive due to this condition}}
 // expected-error@+2 {{increment expression must cause 'i' to increase on each iteration of OpenMP for loop}}
 #pragma omp for
Index: lib/Sema/SemaOpenMP.cpp
===
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -5133,7 +5133,9 @@
   llvm::MapVector Captures;
   SmallVector IterSpaces;
   IterSpaces.resize(NestedLoopCount);
-  Stmt *CurStmt = AStmt->IgnoreContainers(/* IgnoreCaptured */ true);
+  Stmt *CurStmt = AStmt;
+  if (auto CapS = dyn_cast_or_null(CurStmt))
+CurStmt = CapS->getCapturedStmt();
   for (unsigned Cnt = 0; Cnt < NestedLoopCount; ++Cnt) {
 if (CheckOpenMPIterationSpace(DKind, CurStmt, SemaRef, DSA, Cnt,
   NestedLoopCount, CollapseLoopCountExpr,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24615: [OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

[PATCH] D24615: [OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

[PATCH] D24615: [OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

Re: [PATCH] D24615: [OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

[PATCH] D24615: [OpenMP] clang doesnt diagnose if there is a lexical block around a for stmt for OpenMP loops. It is technically not allowed in the OpenMP standard

5 matches

Site Navigation

Mail list logo

Footer information