Post-merge fixes

flang/lib/Lower/OpenMP/ClauseProcessor.cpp

@@ -969,7 +969,7 @@ bool ClauseProcessor::processSectionsReduction(
 }
 
 bool ClauseProcessor::processTargetReduction(
-    llvm::SmallVector<const Fortran::semantics::Symbol *> &reductionSymbols)
+    llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> &reductionSymbols)
     const {
   return findRepeatableClause<omp::clause::Reduction>(
       [&](const omp::clause::Reduction &clause,

flang/lib/Lower/OpenMP/ClauseProcessor.h

@@ -127,8 +127,8 @@ class ClauseProcessor {
   bool processSectionsReduction(mlir::Location currentLocation,
                                 mlir::omp::ReductionClauseOps &result) const;
   bool processTargetReduction(
-      llvm::SmallVector<const Fortran::semantics::Symbol *> &reductionSymbols)
-      const;
+      llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
+          &reductionSymbols) const;
   bool processTo(llvm::SmallVectorImpl<DeclareTargetCapturePair> &result) const;
   bool
   processUseDeviceAddr(mlir::omp::UseDeviceClauseOps &result,

flang/lib/Lower/OpenMP/DataSharingProcessor.cpp

@@ -50,11 +50,8 @@ void DataSharingProcessor::processStep3(mlir::Operation *op, bool isLoop) {
     firOpBuilder.setInsertionPointAfter(op);
     insertDeallocs();
   } else {
-    // insert dummy instruction to mark the insertion position
-    mlir::Value undefMarker = firOpBuilder.create<fir::UndefOp>(
-        op->getLoc(), firOpBuilder.getIndexType());
+    mlir::OpBuilder::InsertionGuard guard(firOpBuilder);
     insertDeallocs();
-    firOpBuilder.setInsertionPointAfter(undefMarker.getDefiningOp());
   }
 }
 

flang/lib/Lower/OpenMP/ReductionProcessor.cpp

@@ -72,7 +72,7 @@ ReductionProcessor::ReductionIdentifier ReductionProcessor::getReductionType(
 
 void ReductionProcessor::addReductionSym(
     const omp::clause::Reduction &reduction,
-    llvm::SmallVector<const Fortran::semantics::Symbol *> &symbols) {
+    llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> &symbols) {
   const auto &objectList{std::get<omp::ObjectList>(reduction.t)};
   llvm::transform(objectList, std::back_inserter(symbols),
                   [](const Object &object) { return object.id(); });

flang/lib/Lower/OpenMP/ReductionProcessor.h

@@ -112,7 +112,7 @@ class ReductionProcessor {
 
   static void addReductionSym(
       const omp::clause::Reduction &reduction,
-      llvm::SmallVector<const Fortran::semantics::Symbol *> &symbols);
+      llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> &symbols);
 
   /// Creates an OpenMP reduction declaration and inserts it into the provided
   /// symbol table. The declaration has a constant initializer with the neutral

flang/lib/Lower/OpenMP/Utils.h

@@ -69,8 +69,6 @@ int64_t getCollapseValue(const List<Clause> &clauses);
 Fortran::semantics::Symbol *
 getOmpObjectSymbol(const Fortran::parser::OmpObject &ompObject);
 
-mlir::omp::TargetOp findParentTargetOp(mlir::OpBuilder &builder);
-
 void genObjectList(const ObjectList &objects,
                    Fortran::lower::AbstractConverter &converter,
                    llvm::SmallVectorImpl<mlir::Value> &operands);

flang/lib/Optimizer/Builder/FIRBuilder.cpp

@@ -248,14 +248,45 @@ mlir::Block *fir::FirOpBuilder::getAllocaBlock() {
   if (auto ompOutlineableIface =
           getRegion()
               .getParentOfType<mlir::omp::OutlineableOpenMPOpInterface>()) {
-    return ompOutlineableIface.getAllocaBlock();
+    // omp.parallel can work as a block construct but it can also be a loop
+    // wrapper when part of a composite construct. Make sure it's only treated
+    // as a block if it's not a wrapper.
+    auto parallelOp =
+        llvm::dyn_cast<mlir::omp::ParallelOp>(*ompOutlineableIface);
+    if (!parallelOp || !llvm::isa_and_present<mlir::omp::DistributeOp>(
+                           parallelOp->getParentOp()))
+      return ompOutlineableIface.getAllocaBlock();
   }
+
+  // All allocations associated with an OpenMP loop wrapper must happen outside
+  // of all wrappers.
+  mlir::Operation *currentOp = getRegion().getParentOp();
+  auto wrapperIface =
+      llvm::isa<mlir::omp::LoopNestOp>(currentOp)
+          ? llvm::cast<mlir::omp::LoopWrapperInterface>(
+                currentOp->getParentOp())
+          : llvm::dyn_cast<mlir::omp::LoopWrapperInterface>(currentOp);
+  if (wrapperIface) {
+    // Cannot use LoopWrapperInterface methods here because the whole nest may
+    // not have been created at this point. Manually traverse parents instead.
+    mlir::omp::LoopWrapperInterface lastWrapperOp = wrapperIface;
+    while (true) {
+      if (auto nextWrapper =
+              llvm::dyn_cast_if_present<mlir::omp::LoopWrapperInterface>(
+                  lastWrapperOp->getParentOp()))
+        lastWrapperOp = nextWrapper;
+      else
+        break;
+    }
+    return &lastWrapperOp->getParentRegion()->front();
+  }
+
   if (getRegion().getParentOfType<mlir::omp::DeclareReductionOp>())
     return &getRegion().front();
+
   if (auto accRecipeIface =
-          getRegion().getParentOfType<mlir::acc::RecipeInterface>()) {
+          getRegion().getParentOfType<mlir::acc::RecipeInterface>())
     return accRecipeIface.getAllocaBlock(getRegion());
-  }
 
   return getEntryBlock();
 }
@@ -266,9 +297,15 @@ mlir::Value fir::FirOpBuilder::createTemporaryAlloc(
     llvm::ArrayRef<mlir::NamedAttribute> attrs) {
   assert(!type.isa<fir::ReferenceType>() && "cannot be a reference");
   // If the alloca is inside an OpenMP Op which will be outlined then pin
-  // the alloca here.
-  const bool pinned =
+  // the alloca here. Make sure that an omp.parallel operation that is taking
+  // a loop wrapper role is not detected as outlineable here.
+  auto iface =
       getRegion().getParentOfType<mlir::omp::OutlineableOpenMPOpInterface>();
+  auto parallelOp =
+      iface ? llvm::dyn_cast<mlir::omp::ParallelOp>(*iface) : nullptr;
+  const bool pinned =
+      iface && (!parallelOp || !llvm::isa_and_present<mlir::omp::DistributeOp>(
+                                   parallelOp->getParentOp()));
   mlir::Value temp =
       create<fir::AllocaOp>(loc, type, /*unique_name=*/llvm::StringRef{}, name,
                             pinned, lenParams, shape, attrs);

flang/lib/Optimizer/CodeGen/FIROpPatterns.cpp

@@ -239,9 +239,16 @@ mlir::Value ConvertFIRToLLVMPattern::genBoxAttributeCheck(
 // 3. The first ancestor that is an OpenMP Op or a LLVMFuncOp
 mlir::Block *
 ConvertFIRToLLVMPattern::getBlockForAllocaInsert(mlir::Operation *op) const {
-  if (auto iface = mlir::dyn_cast<mlir::omp::OutlineableOpenMPOpInterface>(op))
-    return iface.getAllocaBlock();
-  if (auto llvmFuncOp = mlir::dyn_cast<mlir::LLVM::LLVMFuncOp>(op))
+  if (auto iface =
+          mlir::dyn_cast<mlir::omp::OutlineableOpenMPOpInterface>(op)) {
+    // omp.parallel can work as a block construct but it can also be a loop
+    // wrapper when it's part of a composite construct. Make sure it's only
+    // treated as a block if it's not a wrapper.
+    auto parallelOp = llvm::dyn_cast<mlir::omp::ParallelOp>(*iface);
+    if (!parallelOp || !llvm::isa_and_present<mlir::omp::DistributeOp>(
+                           parallelOp->getParentOp()))
+      return iface.getAllocaBlock();
+  } else if (auto llvmFuncOp = mlir::dyn_cast<mlir::LLVM::LLVMFuncOp>(op))
     return &llvmFuncOp.front();
   return getBlockForAllocaInsert(op->getParentOp());
 }

flang/lib/Optimizer/Transforms/DoConcurrentConversion.cpp

@@ -99,15 +99,19 @@ class DoConcurrentConversion : public mlir::OpConversionPattern<fir::DoLoopOp> {
                   "constant LB, UB, and step values.");
     }
 
-    llvm::SmallVector<mlir::Value> lowerBound, upperBound, step;
-    lowerBound.push_back(rewriter.clone(*lbOp)->getResult(0));
-    upperBound.push_back(rewriter.clone(*ubOp)->getResult(0));
-    step.push_back(rewriter.clone(*stepOp)->getResult(0));
+    mlir::omp::LoopNestClauseOps clauseOps;
+    clauseOps.loopLBVar.push_back(rewriter.clone(*lbOp)->getResult(0));
+    clauseOps.loopUBVar.push_back(rewriter.clone(*ubOp)->getResult(0));
+    clauseOps.loopStepVar.push_back(rewriter.clone(*stepOp)->getResult(0));
+    clauseOps.loopInclusiveAttr = rewriter.getUnitAttr();
     // ==== TODO (1) End ====
+    auto wsloopOp = rewriter.create<mlir::omp::WsloopOp>(doLoop.getLoc());
+    rewriter.createBlock(&wsloopOp.getRegion());
+    rewriter.setInsertionPoint(
+        rewriter.create<mlir::omp::TerminatorOp>(wsloopOp.getLoc()));
 
-    auto wsLoopOp = rewriter.create<mlir::omp::WsloopOp>(
-        doLoop.getLoc(), lowerBound, upperBound, step);
-    wsLoopOp.setInclusive(true);
+    auto loopNestOp =
+        rewriter.create<mlir::omp::LoopNestOp>(doLoop.getLoc(), clauseOps);
 
     auto outlineableOp =
         mlir::dyn_cast<mlir::omp::OutlineableOpenMPOpInterface>(*parallelOp);
@@ -180,11 +184,11 @@ class DoConcurrentConversion : public mlir::OpConversionPattern<fir::DoLoopOp> {
 
     // Clone the loop's body inside the worksharing construct using the mapped
     // memref values.
-    rewriter.cloneRegionBefore(doLoop.getRegion(), wsLoopOp.getRegion(),
-                               wsLoopOp.getRegion().begin(), mapper);
+    rewriter.cloneRegionBefore(doLoop.getRegion(), loopNestOp.getRegion(),
+                               loopNestOp.getRegion().begin(), mapper);
 
-    mlir::Operation *terminator = wsLoopOp.getRegion().back().getTerminator();
-    rewriter.setInsertionPointToEnd(&wsLoopOp.getRegion().back());
+    mlir::Operation *terminator = loopNestOp.getRegion().back().getTerminator();
+    rewriter.setInsertionPointToEnd(&loopNestOp.getRegion().back());
     rewriter.create<mlir::omp::YieldOp>(terminator->getLoc());
     rewriter.eraseOp(terminator);
 

flang/lib/Optimizer/Transforms/StackArrays.cpp

@@ -571,8 +571,31 @@ AllocMemConversion::findAllocaInsertionPoint(fir::AllocMemOp &oldAlloc) {
     return {point};
   };
 
-  auto oldOmpRegion =
-      oldAlloc->getParentOfType<mlir::omp::OutlineableOpenMPOpInterface>();
+  // Find the first OpenMP outlineable parent region while taking into account
+  // the possibility of finding an omp.parallel region that is taking a loop
+  // wrapper role. These operations must be skipped, as they cannot hold
+  // allocations.
+  const auto findOmpRegion = [](mlir::Operation *op) {
+    auto findOmpRegionImpl =
+        [](mlir::Operation *op,
+           auto &findOmpRegion) -> mlir::omp::OutlineableOpenMPOpInterface {
+      auto ompRegion =
+          op->getParentOfType<mlir::omp::OutlineableOpenMPOpInterface>();
+      if (!ompRegion)
+        return nullptr;
+
+      if (auto parallelOp =
+              mlir::dyn_cast_if_present<mlir::omp::ParallelOp>(*ompRegion)) {
+        mlir::Operation *parentOp = parallelOp->getParentOp();
+        if (mlir::isa_and_present<mlir::omp::DistributeOp>(parentOp))
+          return findOmpRegion(parentOp, findOmpRegion);
+      }
+      return ompRegion;
+    };
+    return findOmpRegionImpl(op, findOmpRegionImpl);
+  };
+
+  auto oldOmpRegion = findOmpRegion(oldAlloc);
 
   // Find when the last operand value becomes available
   mlir::Block *operandsBlock = nullptr;
@@ -600,8 +623,7 @@ AllocMemConversion::findAllocaInsertionPoint(fir::AllocMemOp &oldAlloc) {
     LLVM_DEBUG(llvm::dbgs()
                << "--Placing after last operand: " << *lastOperand << "\n");
     // check we aren't moving out of an omp region
-    auto lastOpOmpRegion =
-        lastOperand->getParentOfType<mlir::omp::OutlineableOpenMPOpInterface>();
+    auto lastOpOmpRegion = findOmpRegion(lastOperand);
     if (lastOpOmpRegion == oldOmpRegion)
       return checkReturn(lastOperand);
     // Presumably this happened because the operands became ready before the

flang/test/Fir/convert-to-llvm-openmp-and-fir.fir

@@ -199,6 +199,7 @@ func.func @_QPsimd1(%arg0: !fir.ref<i32> {fir.bindc_name = "n"}, %arg1: !fir.ref
         fir.store %3 to %6 : !fir.ref<i32>
         omp.yield
       }
+      omp.terminator
     }
     omp.terminator
   }
@@ -223,6 +224,7 @@ func.func @_QPsimd1(%arg0: !fir.ref<i32> {fir.bindc_name = "n"}, %arg1: !fir.ref
 // CHECK:   llvm.store %[[I1]], %[[ARR_I_REF]] : i32, !llvm.ptr
 // CHECK: omp.yield
 // CHECK: }
+// CHECK: omp.terminator
 // CHECK: }
 // CHECK: omp.terminator
 // CHECK: }
@@ -516,6 +518,7 @@ func.func @_QPsimd_with_nested_loop() {
       fir.store %7 to %3 : !fir.ref<i32>
       omp.yield
     }
+    omp.terminator
   }
   return
 }
@@ -536,6 +539,7 @@ func.func @_QPsimd_with_nested_loop() {
 // CHECK:             ^bb3:
 // CHECK:               omp.yield
 // CHECK:             }
+// CHECK:             omp.terminator
 // CHECK:           }
 // CHECK:           llvm.return
 // CHECK:         }