[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: clang/include/clang/AST/StmtOpenMP.h:41
+/// purposes:
+/// * Loop variable: The user-accessible variable with different value for each
+///   iteration.
----------------
jdenny wrote:
> I suggest the terms "loop user variable" and "loop iteration variable".  In particular, I find "loop counter" to be misleading because "counter" suggests to me it's always an integer, so it's easy to get it confused with the "logical iteration counter".  If you're using standard terminology that I'm not aware of, then never mind.
Good suggestion


================
Comment at: clang/include/clang/AST/StmtOpenMP.h:92
+/// <code>
+///   [&,__begin](std::vector<std::string>::iterator &Result, size_t Logical) {
+///   Result = __begin + Logical; }
----------------
jdenny wrote:
> Why is `__begin` an explicit capture here but not for the distance function?
Because `__begin` is captured by-value, everything lese uses the `&`default capture. This is the loop counter variable is modified inside the loop body.


================
Comment at: clang/include/clang/AST/StmtOpenMP.h:101
+  enum {
+    LOOPY_STMT,
+    DISTANCE_FUNC,
----------------
jdenny wrote:
> Why "loopy" with a "y"?
`loopy` was meant to refer to the property of the statement (ForStmt, CXXForRangeStmt, potentially others such as WhileStmt, DoStmt, functions from `#include <algorithm>`, etc) instead of a specific loop node (such as OMPLoopDirective or OMPCanonicalLoop itself), although I did not apply this idea consistently. Do you prefer a plain 'LOOP_STMT'?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94973