[flang-commits] [PATCH] D149878: [flang][hlfir] Lower forall to HLFIR

[Jean Perier via Phabricator via flang-commits]

jeanPerier marked an inline comment as done.
jeanPerier added inline comments.


================
Comment at: flang/lib/Lower/Bridge.cpp:2048
+      mlir::OpBuilder::InsertPoint innerInsertionPoint;
+      if (outerForall && !isMask) {
+        innerInsertionPoint = builder->saveInsertionPoint();
----------------
vzakhari wrote:
> Why cannot we do the same for the masked case?
Because it may, and most likely will, depend on the forall indices being defined in the concurrent header.
Typically, this will be `forall(i=lb:ub, some_predicate(i)) x(i) = y(i)`. These symbols depends on the hlfir.forall block arguments, so the expression can only be evaluated inside it.

We could check if the mask expression depends on the forall symbols via CollectSymbols, but I think we would be wasting compilation time because I do not think it is usual to have a scalar mask that does not depend on the indices (it would be equivalent to having "if (cdt) { forall {} })".

Note that the bounds/step expressions of forall indices in the same forall may not depend on any of the indices of the same concurrent header (Fortran 2018 C1123 constraints), so it is safe to outline them.

I added a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149878