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

Richard Smith richard at metafoo.co.uk
Mon Jun 2 17:29:13 PDT 2014


================
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;
+  }
+
----------------
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.

================
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.
----------------
Should this have a name involving OpenMP? It's not obvious from the declaration that this isn't a general-purpose utility.

================
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));
+      }
+    }
----------------
No braces around a single-line `if` body.

================
Comment at: lib/Sema/SemaOpenMP.cpp:1270-1271
@@ +1269,4 @@
+
+  if (ISC.Dependent())
+    return false;
+
----------------
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`.

http://reviews.llvm.org/D3778






More information about the cfe-commits mailing list