[flang][Lower][nfc] vector subscript lhs first element to helper

[tblah]

This encapsulates implementation details of hlfir.elemental_addr inside of ConvertExprToHLFIR instead of leaking to OpenMP code.

Requested here:
#133892 (comment)

[llvmbot]

@llvm/pr-subscribers-flang-fir-hlfir

Author: Tom Eccles (tblah)

Changes

This encapsulates implementation details of hlfir.elemental_addr inside of ConvertExprToHLFIR instead of leaking to OpenMP code.

Requested here:
#133892 (comment)

---

Full diff: https://github.com/llvm/llvm-project/pull/137456.diff

3 Files Affected:

• (modified) flang/include/flang/Lower/ConvertExprToHLFIR.h (+9)
• (modified) flang/lib/Lower/ConvertExprToHLFIR.cpp (+46)
• (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+2-44)

diff --git a/flang/include/flang/Lower/ConvertExprToHLFIR.h b/flang/include/flang/Lower/ConvertExprToHLFIR.h
index c64fb01b33ed9..f4d7701e50a2c 100644
--- a/flang/include/flang/Lower/ConvertExprToHLFIR.h
+++ b/flang/include/flang/Lower/ConvertExprToHLFIR.h
@@ -137,6 +137,15 @@ hlfir::ElementalAddrOp convertVectorSubscriptedExprToElementalAddr(
     const Fortran::lower::SomeExpr &, Fortran::lower::SymMap &,
     Fortran::lower::StatementContext &);
 
+/// Lower a designator containing vector subscripts, creating a hlfir::Entity
+/// representing the first element in the vector subscripted array. This is a
+/// helper which calls convertVectorSubscriptedExprToElementalAddr and lowers
+/// the hlfir::ElementalAddrOp.
+hlfir::Entity genVectorSubscriptedDesignatorFirstElementAddress(
+    mlir::Location loc, Fortran::lower::AbstractConverter &converter,
+    const Fortran::lower::SomeExpr &expr, Fortran::lower::SymMap &symMap,
+    Fortran::lower::StatementContext &stmtCtx);
+
 } // namespace Fortran::lower
 
 #endif // FORTRAN_LOWER_CONVERTEXPRTOHLFIR_H
diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index 5a4521140045c..a1a2d3a169362 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -31,6 +31,7 @@
 #include "flang/Optimizer/Builder/Runtime/Pointer.h"
 #include "flang/Optimizer/Builder/Todo.h"
 #include "flang/Optimizer/HLFIR/HLFIROps.h"
+#include "mlir/IR/IRMapping.h"
 #include "llvm/ADT/TypeSwitch.h"
 #include <optional>
 
@@ -2120,3 +2121,48 @@ Fortran::lower::convertVectorSubscriptedExprToElementalAddr(
   return HlfirDesignatorBuilder(loc, converter, symMap, stmtCtx)
       .convertVectorSubscriptedExprToElementalAddr(designatorExpr);
 }
+
+hlfir::Entity Fortran::lower::genVectorSubscriptedDesignatorFirstElementAddress(
+    mlir::Location loc, Fortran::lower::AbstractConverter &converter,
+    const Fortran::lower::SomeExpr &expr, Fortran::lower::SymMap &symMap,
+    Fortran::lower::StatementContext &stmtCtx) {
+  fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+
+  // Get a hlfir.elemental_addr op describing the address of the value
+  // indexed from the original array.
+  // Note: the hlfir.elemental_addr op verifier requires it to be inside
+  // of a hlfir.region_assign op. This operation is never seen by the
+  // verifier because it is immediately inlined.
+  hlfir::ElementalAddrOp addrOp = convertVectorSubscriptedExprToElementalAddr(
+      loc, converter, expr, symMap, stmtCtx);
+  if (!addrOp.getCleanup().empty())
+    TODO(converter.getCurrentLocation(),
+         "Vector subscript requring a cleanup region");
+
+  // hlfir.elemental_addr doesn't have a normal lowering because it
+  // can't return a value. Instead we need to inline it here using
+  // values for the first element. Similar to hlfir::inlineElementalOp.
+
+  mlir::Value one = builder.createIntegerConstant(
+      converter.getCurrentLocation(), builder.getIndexType(), 1);
+  mlir::SmallVector<mlir::Value> oneBasedIndices;
+  oneBasedIndices.resize(addrOp.getIndices().size(), one);
+
+  mlir::IRMapping mapper;
+  mapper.map(addrOp.getIndices(), oneBasedIndices);
+  assert(addrOp.getElementalRegion().hasOneBlock());
+  mlir::Operation *newOp;
+  for (mlir::Operation &op : addrOp.getElementalRegion().back().getOperations())
+    newOp = builder.clone(op, mapper);
+  auto yield = mlir::cast<hlfir::YieldOp>(newOp);
+
+  addrOp->erase();
+
+  if (!yield.getCleanup().empty())
+    TODO(converter.getCurrentLocation(),
+         "Vector subscript requring element cleanup");
+
+  hlfir::Entity result{yield.getEntity()};
+  yield->erase();
+  return result;
+}
\ No newline at end of file
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index cce6dc32bad4b..77b4622547d7a 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -895,50 +895,8 @@ bool ClauseProcessor::processDepend(lower::SymMap &symMap,
           // the standard to be the lowest index) to identify the dependency. We
           // don't need an accurate length for the array section because the
           // OpenMP standard forbids overlapping array sections.
-
-          // Get a hlfir.elemental_addr op describing the address of the value
-          // indexed from the original array.
-          // Note: the hlfir.elemental_addr op verifier requires it to be inside
-          // of a hlfir.region_assign op. This is because the only place in base
-          // Fortran where you need the address of a vector subscript would be
-          // in an assignment operation. We are not doing an assignment here
-          // but we do want the address (without having to duplicate all of
-          // Fortran designation lowering!). This operation is never seen by the
-          // verifier because it is immediately inlined.
-          hlfir::ElementalAddrOp addrOp =
-              convertVectorSubscriptedExprToElementalAddr(
-                  converter.getCurrentLocation(), converter, expr, symMap,
-                  stmtCtx);
-          if (!addrOp.getCleanup().empty())
-            TODO(converter.getCurrentLocation(),
-                 "Vector subscript in DEPEND clause requring a cleanup region");
-
-          // hlfir.elemental_addr doesn't have a normal lowering because it
-          // can't return a value. Instead we need to inline it here using
-          // values for the first element. Similar to hlfir::inlineElementalOp.
-
-          mlir::Value one = builder.createIntegerConstant(
-              converter.getCurrentLocation(), builder.getIndexType(), 1);
-          mlir::SmallVector<mlir::Value> oneBasedIndices;
-          oneBasedIndices.resize(addrOp.getIndices().size(), one);
-
-          mlir::IRMapping mapper;
-          mapper.map(addrOp.getIndices(), oneBasedIndices);
-          assert(addrOp.getElementalRegion().hasOneBlock());
-          mlir::Operation *newOp;
-          for (mlir::Operation &op :
-               addrOp.getElementalRegion().back().getOperations())
-            newOp = builder.clone(op, mapper);
-          auto yield = mlir::cast<hlfir::YieldOp>(newOp);
-
-          addrOp->erase();
-
-          if (!yield.getCleanup().empty())
-            TODO(converter.getCurrentLocation(),
-                 "Vector subscript in DEPEND clause requring element cleanup");
-
-          dependVar = yield.getEntity();
-          yield->erase();
+          dependVar = genVectorSubscriptedDesignatorFirstElementAddress(
+              converter.getCurrentLocation(), converter, expr, symMap, stmtCtx);
         } else {
           // Ordinary array section e.g. A(1:512:2)
           hlfir::EntityWithAttributes entity = convertExprToHLFIR(

[jeanPerier]

Thanks for following up Tom, LGTM

[jeanPerier]

Nit: missing new line