[PATCH] D90830: [OpenMPIRBuilder] Implement CreateCanonicalLoop.

[Fady Ghanim via Phabricator via llvm-commits]

fghanim added a comment.

Thanks for the Patch. Generally looks Good.
Just a couple of very minor comments/questions

- My understanding of the usage case for this, is that when we get `CreateDistribute`, `CreateSimd`, `CreateLoop`, etc., they will use this to generate the loop/s required, but this is not meant to be used directly by clang or flang? is that correct? If yes, why do we have them as public methods, wouldn't it be better to make them private?

- Also, a question; I think when we have something like:

  #pragma omp for
  for ( ... ; ... ; ...) {
   //body
  }

this is going to be encoded in the clang AST as the `omp for` statement, whose body the `ForStatement` node. Now the first will end up using the `CreateForDirective()` that we plan to add in the OMPBuilder, while the latter will basically have clang generate the loop for us ( i.e. using`
`emitForStatement()` ). is this correct? If yes, wouldn't that mean that there would be some redundancy, or is this for a different case?



================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1403
+  assert(Cond->getSinglePredecessor() == Header);
+  assert(isa<BranchInst>(Header->getTerminator()));
+  assert(size(successors(Cond)) == 2);
----------------
did you mean to say `Cond->getTerminator()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90830