[PATCH] D112499: [OpenMP] Initial parsing/sema for the 'omp loop' construct

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Basic/OpenMPKinds.cpp:683
+  case OMPD_loop:
+    CaptureRegions.push_back(OMPD_loop);
+    break;
----------------
mikerice wrote:
> ABataev wrote:
> > Why it cannot be just `OMPD_unknown` here just like for `omp for` directive? Should it be outlined as a function?
> I didn't write any codegen for this but my impression was that 'loop' can be treated like a 'parallel for' depending on the binding/situation and need outlining. That's why I did this. If that isn't the right assumption I can change it. 
Hmm, not sure if this is correct. Could you clarify a bit on the expected codegen for different kinds of bindings? Some examples in pseudocode, maybe?


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:397
+  case Stmt::OMPGenericLoopDirectiveClass:
+    llvm_unreachable("codegen for loop directive not yet implemented");
   }
----------------
mikerice wrote:
> ABataev wrote:
> > Better just to emit it as an inlined directive.
> Not sure exactly what you mean by inlined, but if you have something trivial for me to add.  I wasn't planning to implement any codegen in this patch.
I just meant that if we can emit the associated statement as is, better to do it.
```
auto &&CodeGen = [&S](CodeGenFunction &CGF, PrePostActionTy &Action) {
  CGF.EmitStmt(S.getAssociatedStmt());
};
OMPLexicalScope Scope(*this, S, OMPD_unknown);
CGM.getOpenMPRuntime().emitInlinedDirective(*this, OMPD_loop, CodeGen);
```
or something like this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112499/new/

https://reviews.llvm.org/D112499