[PATCH] [OPENMP] Loop canonical form analysis (Sema)

[Alexander Musman]

================
Comment at: include/clang/Sema/Scope.h:399-406
@@ +398,10 @@
+
+  /// \brief Determine whether this scope is nested into some OpenMP simd
+  /// directive scope.
+  bool isOpenMPSimdNestedScope() const {
+    for (const Scope *S = getParent(); S; S = S->getParent())
+      if (S->isOpenMPSimdDirectiveScope())
+        return true;
+    return false;
+  }
+
----------------
Richard Smith wrote:
> Please either handle this by copying the `OpenMPSimdDirectiveScope` to relevant child scopes in `Scope::Init` or by adding an `OpenMPSimdDirectiveParent` (if you actually need to know which parent scope had the directive).
> 
> Also, this loop doesn't look entirely correct: I would expect that simd-ness shouldn't propagate into (say) local classes declared within an OpenMP simd directive:
> 
>   void f() {
>     #pragma omp simd ...
>     for (...) {
>       struct S {
>         void g() { throw 0; }
>       };
>     }
>   }
> 
> ... but I'm not sure what the OpenMP spec says about this case.
The OpenMP spec does not cover such cases, but yes, it seems reasonable to allow EH here.

================
Comment at: include/clang/Sema/Sema.h:7264
@@ -7264,2 +7263,3 @@
 public:
+  ExprResult PerformImplicitIntegerConversion(SourceLocation OpLoc, Expr *Op);
   /// \brief Called on start of new data sharing attribute block.
----------------
Richard Smith wrote:
> Should this have a name involving OpenMP? It's not obvious from the declaration that this isn't a general-purpose utility.
OK, I've renamed it to PerformOpenMPImplicitIntegerConversion

================
Comment at: lib/Sema/SemaOpenMP.cpp:1018-1020
@@ +1017,5 @@
+    if (CE->getOperator() == OO_Equal) {
+      if (auto DRE = dyn_cast<DeclRefExpr>(CE->getArg(0))) {
+        return SetVarAndLB(dyn_cast<VarDecl>(DRE->getDecl()), CE->getArg(1));
+      }
+    }
----------------
Richard Smith wrote:
> No braces around a single-line `if` body.
Fixed

================
Comment at: lib/Sema/SemaOpenMP.cpp:1270-1271
@@ +1269,4 @@
+
+  if (ISC.Dependent())
+    return false;
+
----------------
Richard Smith wrote:
> What's this for? If `HasErrors` is true here, then we've found and diagnosed a problem despite some part of the loop being dependent, and we should return `true`.
I agree, fixed.

http://reviews.llvm.org/D3778