review comments

Author: schweitzpgi

flang/include/flang/Lower/ConvertExpr.h

@@ -170,6 +170,9 @@ createSomeArrayTempValue(AbstractConverter &converter,
                          const evaluate::Expr<evaluate::SomeType> &expr,
                          SymMap &symMap, StatementContext &stmtCtx);
 
+/// Like createSomeArrayTempValue, but the temporary buffer is allocated lazily
+/// (inside the loops instead of before the loops). This can be useful if a
+/// loop's bounds are functions of other loop indices, for example.
 fir::ExtendedValue
 createLazyArrayTempValue(AbstractConverter &converter,
                          const evaluate::Expr<evaluate::SomeType> &expr,

flang/lib/Lower/Bridge.cpp

@@ -2682,12 +2682,9 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     explicitIterSpace.outermostContext().attachCleanup([=]() {
       auto load = builder->create<fir::LoadOp>(loc, var);
       auto cmp = builder->genIsNotNull(loc, load);
-      auto ifOp =
-          builder->create<fir::IfOp>(loc, cmp, /*withElseRegion=*/false);
-      auto insPt = builder->saveInsertionPoint();
-      builder->setInsertionPointToStart(&ifOp.thenRegion().front());
-      builder->create<fir::FreeMemOp>(loc, load);
-      builder->restoreInsertionPoint(insPt);
+      builder->genIfThen(loc, cmp)
+          .genThen([&]() { builder->create<fir::FreeMemOp>(loc, load); })
+          .end();
     });
   }
 

flang/lib/Lower/ConvertExpr.cpp

@@ -3356,7 +3356,7 @@ class ArrayExprLowering {
     auto loc = getLoc();
     // Once the loop extents have been computed, which may require being inside
     // some explicit loops, lazily allocate the expression on the heap.
-    ccPrelude = [=](llvm::ArrayRef<mlir::Value> shape) -> mlir::Value {
+    ccPrelude = [=](llvm::ArrayRef<mlir::Value> shape) {
       auto load = builder.create<fir::LoadOp>(loc, var);
       auto eleTy = fir::unwrapRefType(load.getType());
       auto unknown = fir::SequenceType::getUnknownExtent();
@@ -3365,15 +3365,14 @@ class ArrayExprLowering {
       auto toTy = fir::HeapType::get(seqTy);
       auto castTo = builder.createConvert(loc, toTy, load);
       auto cmp = builder.genIsNull(loc, castTo);
-      auto ifOp = builder.create<fir::IfOp>(loc, cmp, /*withElseRegion=*/false);
-      auto insPt = builder.saveInsertionPoint();
-      builder.setInsertionPointToStart(&ifOp.thenRegion().front());
-      auto mem = builder.create<fir::AllocMemOp>(loc, seqTy, ".lazy.mask",
-                                                 llvm::None, shape);
-      auto uncast = builder.createConvert(loc, load.getType(), mem);
-      builder.create<fir::StoreOp>(loc, uncast, var);
-      builder.restoreInsertionPoint(insPt);
-      return mem;
+      builder.genIfThen(loc, cmp)
+          .genThen([&]() {
+            auto mem = builder.create<fir::AllocMemOp>(loc, seqTy, ".lazy.mask",
+                                                       llvm::None, shape);
+            auto uncast = builder.createConvert(loc, load.getType(), mem);
+            builder.create<fir::StoreOp>(loc, uncast, var);
+          })
+          .end();
     };
     // Create a dummy array_load before the loop. We're storing to a lazy
     // temporary, so there will be no conflict and no copy-in.
@@ -3603,7 +3602,7 @@ class ArrayExprLowering {
       // Mask expressions are array expressions too.
       for (const auto *e : implicitSpace->getExprs())
         if (e && !implicitSpace->isLowered(e)) {
-          if (auto var = implicitSpace->lookupVariable(e)) {
+          if (auto var = implicitSpace->lookupMaskVariable(e)) {
             // Allocate the mask buffer lazily.
             auto tmp = Fortran::lower::createLazyArrayTempValue(
                 converter, *e, var, symMap, stmtCtx);
@@ -3660,10 +3659,8 @@ class ArrayExprLowering {
     llvm::SmallVector<mlir::Value> ivars;
     if (loopDepth > 0) {
       // Generate the lazy mask allocation, if one was given.
-      if (ccPrelude.hasValue()) {
-        [[maybe_unused]] auto allocMem = ccPrelude.getValue()(shape);
-        assert(allocMem && "mask buffer allocation failure");
-      }
+      if (ccPrelude.hasValue())
+        ccPrelude.getValue()(shape);
 
       auto *startBlock = builder.getBlock();
       for (auto i : llvm::enumerate(llvm::reverse(loopUppers))) {
@@ -5540,8 +5537,7 @@ class ArrayExprLowering {
   Fortran::lower::SymMap &symMap;
   /// The continuation to generate code to update the destination.
   llvm::Optional<CC> ccStoreToDest;
-  llvm::Optional<std::function<mlir::Value(llvm::ArrayRef<mlir::Value>)>>
-      ccPrelude;
+  llvm::Optional<std::function<void(llvm::ArrayRef<mlir::Value>)>> ccPrelude;
   llvm::Optional<std::function<fir::ArrayLoadOp(llvm::ArrayRef<mlir::Value>)>>
       ccLoadDest;
   /// The destination is the loaded array into which the results will be

flang/lib/Lower/IterationSpace.h

@@ -127,7 +127,7 @@ class ImplicitIterSpace
     maskVarMap.try_emplace(exp, var);
   }
 
-  mlir::Value lookupVariable(FrontEndExpr exp) {
+  mlir::Value lookupMaskVariable(FrontEndExpr exp) {
     return maskVarMap.lookup(exp);
   }