[Flang][OpenMP] Refactor to properly fix privatisation of loop bounds

[kiranchandramohan]

Summary

The OpenMP loop Operations have the bounds attached to them. If the
loop bounds are privatised then the privatisation has to happen
before the loop operation is created. To do this the privatisation
is split into two steps. The first step performs cloning and
firstprivate handling, the second step performs lastprivate handling.

This also reverts the changes in the temporary fix (D127137).

Fixes https://github.com/flang-compiler/f18-llvm-project/issues/1171#issuecomment-1143880545
Fixes https://github.com/flang-compiler/f18-llvm-project/issues/1171#issuecomment-1119997442

Fixes #60872

[NimishMishra]

LGTM

flang/lib/Lower/OpenMP.cpp

102

Nit: "all privatisation clauses" instead of "al ..."

112

Not a hard comment but does it make more sense to name these steps? Rather than leaving them as "Step1" and "Step2" ? Like "Step2" can be "processLastPrivateClause" or something.

Again, this is purely from readability perspective. In any case, the comment here clearly distinguishes Step1 and Step2.

1555

Unintentional?

1571

This looks OK. Splitting privatization in steps 1 and 2 have cleanly allowed privatization of loop variables before creation of loop operations (step 1). Lastprivate is left for later (step 2) when we have the MLIR operation information.

flang/test/Lower/OpenMP/single.f90

75–77

This change is OK. private(x) firstprivate(y) should cuase the allocations within omp.single code block, which wasn't really happening before.

[kiranchandramohan]

Thanks @NimishMishra for the review.

flang/lib/Lower/OpenMP.cpp

112

Thanks, that is a good point.
I will do this in further refactorings or cleanups. The plan is to come to the following steps. And at that point, we can do the renaming.
-> clone
-> copyfirstprivate
-> copylastprivate
-> finalize (for allocatables, or derived types with finalizers)

flang/test/Lower/OpenMP/single.f90

75–77

I had previously suggested hoisting them. But I felt this is better for simplicity. But going forward there is the following further work,

1. Provide an option to hoist allocas to the nearest region that will be function-like.
2. Insert llvm.stack-save and llvm.stack-restore so that if the single is inside a loop it does not cause runaway allocation.

flang/lib/Lower/Bridge.cpp

fir::ExtendedValue exv = hexv.match(
         },
         [&](const auto &) -> fir::ExtendedValue {
           mlir::Value temp =
               allocate(fir::factory::getExtents(loc, *builder, hexv),
                        fir::factory::getTypeParams(loc, *builder, hexv));
           return fir::substBase(hexv, temp);
         });
 
-    // Replace all uses of the original with the clone/copy,
-    // esepcially for loop bounds (that uses the variable being privatised)
-    // since loop bounds use old values that need to be fixed by using the
-    // new copied value.
-    // Not able to use replaceAllUsesWith() because uses outside
-    // the loop body should not use the clone.
-    // FIXME: Call privatization before the loop operation.
-    mlir::Region &curRegion = getFirOpBuilder().getRegion();
-    mlir::Value oldVal = fir::getBase(hexv);
-    mlir::Value cloneVal = fir::getBase(exv);
-    for (auto &oper : curRegion.getOps()) {
-      for (unsigned int ii = 0; ii < oper.getNumOperands(); ++ii) {
-        if (oper.getOperand(ii) == oldVal) {
-          oper.setOperand(ii, cloneVal);
-        }
-      }
-    }
     return bindIfNewSymbol(sym, exv);
   }
 
   void copyHostAssociateVar(
       const Fortran::semantics::Symbol &sym,
       mlir::OpBuilder::InsertPoint *copyAssignIP = nullptr) override final {
     // 1) Fetch the original copy of the variable.
     assert(sym.has<Fortran::semantics::HostAssocDetails>() &&

flang/lib/Lower/OpenMP.cpp

class DataSharingProcessor {
   bool hasLastPrivateOp;
   mlir::OpBuilder::InsertPoint lastPrivIP;
   mlir::OpBuilder::InsertPoint insPt;
   // Symbols in private, firstprivate, and/or lastprivate clauses.
   llvm::SetVector<const Fortran::semantics::Symbol *> privatizedSymbols;
   llvm::SetVector<const Fortran::semantics::Symbol *> defaultSymbols;
   llvm::SetVector<const Fortran::semantics::Symbol *> symbolsInNestedRegions;
   llvm::SetVector<const Fortran::semantics::Symbol *> symbolsInParentRegions;
-  mlir::Operation *op;
   Fortran::lower::AbstractConverter &converter;
   fir::FirOpBuilder &firOpBuilder;
   const Fortran::parser::OmpClauseList &opClauseList;
   Fortran::lower::pft::Evaluation &eval;
 
-  void privatizeSymbol(
-      const Fortran::semantics::Symbol *sym,
-      [[maybe_unused]] mlir::OpBuilder::InsertPoint *lastPrivIP = nullptr);
   bool needBarrier();
   void collectSymbols(Fortran::semantics::Symbol::Flag flag);
   void collectOmpObjectListSymbol(
       const Fortran::parser::OmpObjectList &ompObjectList,
       llvm::SetVector<const Fortran::semantics::Symbol *> &symbolSet);
   void collectSymbolsForPrivatization();
   void insertBarrier();
   void collectDefaultSymbols();
   void privatize();
   void defaultPrivatize();
+  void copyLastPrivatize(mlir::Operation *op);
   void insertLastPrivateCompare(mlir::Operation *op);
+  void cloneSymbol(const Fortran::semantics::Symbol *sym);
+  void copyFirstPrivateSymbol(const Fortran::semantics::Symbol *sym);
+  void copyLastPrivateSymbol(const Fortran::semantics::Symbol *sym,
+                             mlir::OpBuilder::InsertPoint *lastPrivIP);
 
 public:
-  DataSharingProcessor(mlir::Operation *op,
-                       Fortran::lower::AbstractConverter &converter,
+  DataSharingProcessor(Fortran::lower::AbstractConverter &converter,
                        const Fortran::parser::OmpClauseList &opClauseList,
                        Fortran::lower::pft::Evaluation &eval)
-      : hasLastPrivateOp(false), op(op), converter(converter),
+      : hasLastPrivateOp(false), converter(converter),
         firOpBuilder(converter.getFirOpBuilder()), opClauseList(opClauseList),
         eval(eval) {}
-  bool process();
+  // Privatisation is split into two steps.
+  // Step1 performs cloning of all privatisation clauses and copying for
+  // firstprivates. Step1 is performed at the place where process/processStep1
+  // is called. This is usually inside the Operation corresponding to the OpenMP
+  // construct, for looping constructs this is just before the Operation. The
+  // split into two steps was performed basically to be able to call
+  // privatisation for looping constructs before the operation is created since
+  // the bounds of the MLIR OpenMP operation can be privatised. Step2 performs
+  // the copying for lastprivates. Step2 requires knowledge of the MLIR
+  // operation to insert the last private update.
+  bool process(mlir::Operation *op);
+  void processStep1();
+  bool processStep2(mlir::Operation *op);
 };
 
-bool DataSharingProcessor::process() {
-  insPt = firOpBuilder.saveInsertionPoint();
-  collectSymbolsForPrivatization();
-  insertLastPrivateCompare(op);
-  if (mlir::isa<mlir::omp::SectionOp>(op)) {
-    if (!eval.lowerAsUnstructured())
-      firOpBuilder.setInsertionPointToStart(&op->getRegion(0).back());
-  } else {
-    firOpBuilder.setInsertionPointToStart(firOpBuilder.getAllocaBlock());
-  }
-  privatize();
+bool DataSharingProcessor::process(mlir::Operation *op) {
+  processStep1();
+  assert(op && "Current MLIR operation not set");
+  return processStep2(op);
+
+void DataSharingProcessor::processStep1() {
+  collectSymbolsForPrivatization();
   collectDefaultSymbols();
+  privatize();
   defaultPrivatize();
   insertBarrier();
+}
+
+bool DataSharingProcessor::processStep2(mlir::Operation *op) {
+  insPt = firOpBuilder.saveInsertionPoint();
+  copyLastPrivatize(op);
   firOpBuilder.restoreInsertionPoint(insPt);
   return hasLastPrivateOp;
 }
 
-void DataSharingProcessor::privatizeSymbol(
-    const Fortran::semantics::Symbol *sym,
-    [[maybe_unused]] mlir::OpBuilder::InsertPoint *lastPrivIP) {
+void DataSharingProcessor::cloneSymbol(const Fortran::semantics::Symbol *sym) {
   // Privatization for symbols which are pre-determined (like loop index
   // variables) happen separately, for everything else privatize here.
   if (sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined))
     return;
   bool success = converter.createHostAssociateVarClone(*sym);
   (void)success;
   assert(success && "Privatization failed due to existing binding");
-  if (sym->test(Fortran::semantics::Symbol::Flag::OmpFirstPrivate)) {
-    fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
-    mlir::OpBuilder::InsertPoint firstPrivIP, insPt;
-    if (mlir::isa<mlir::omp::SingleOp>(op)) {
-      insPt = firOpBuilder.saveInsertionPoint();
-      firOpBuilder.setInsertionPointToStart(&op->getRegion(0).front());
-      firstPrivIP = firOpBuilder.saveInsertionPoint();
-    }
-    converter.copyHostAssociateVar(*sym, &firstPrivIP);
-    if (mlir::isa<mlir::omp::SingleOp>(op))
-      firOpBuilder.restoreInsertionPoint(insPt);
-  }
+
+void DataSharingProcessor::copyFirstPrivateSymbol(
+    const Fortran::semantics::Symbol *sym) {
+  if (sym->test(Fortran::semantics::Symbol::Flag::OmpFirstPrivate))
+    converter.copyHostAssociateVar(*sym);
+
+void DataSharingProcessor::copyLastPrivateSymbol(
+    const Fortran::semantics::Symbol *sym,
+    [[maybe_unused]] mlir::OpBuilder::InsertPoint *lastPrivIP) {
   if (sym->test(Fortran::semantics::Symbol::Flag::OmpLastPrivate))
     converter.copyHostAssociateVar(*sym, lastPrivIP);
 }
 
 void DataSharingProcessor::collectOmpObjectListSymbol(
     const Fortran::parser::OmpObjectList &ompObjectList,
     llvm::SetVector<const Fortran::semantics::Symbol *> &symbolSet) {
   for (const Fortran::parser::OmpObject &ompObject : ompObjectList.v) {
void DataSharingProcessor ::insertBarrier() {
   // Emit implicit barrier for linear clause. Maybe on somewhere else.
   if (needBarrier())
     firOpBuilder.create<mlir::omp::BarrierOp>(converter.getCurrentLocation());
 }
 
 void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
   mlir::arith::CmpIOp cmpOp;
   bool cmpCreated = false;
+  mlir::OpBuilder::InsertPoint localInsPt = firOpBuilder.saveInsertionPoint();
   for (const Fortran::parser::OmpClause &clause : opClauseList.v) {
     if (std::get_if<Fortran::parser::OmpClause::Lastprivate>(&clause.u)) {
       // TODO: Add lastprivate support for simd construct
       if (mlir::isa<omp::SectionOp>(op)) {
         if (&eval == &eval.parentConstruct->getLastNestedEvaluation()) {
           // For `omp.sections`, lastprivatized variables occur in
           // lexically final `omp.section` operation. The following FIR
           // shall be generated for the same:
if (std::get_if<Fortran::parser::OmpClause::Lastprivate>(&clause.u)) {
         lastPrivIP = firOpBuilder.saveInsertionPoint();
       } else {
         TODO(converter.getCurrentLocation(),
              "lastprivate clause in constructs other than "
              "simd/worksharing-loop");
       }
     }
   }
+  firOpBuilder.restoreInsertionPoint(localInsPt);
 }
 
 void DataSharingProcessor::collectSymbols(
     Fortran::semantics::Symbol::Flag flag) {
   converter.collectSymbolSet(eval, defaultSymbols, flag,
                              /*collectSymbols=*/true,
                              /*collectHostAssociatedSymbols=*/true);
   for (auto &e : eval.getNestedEvaluations()) {
if (const auto &defaultClause =
       else if (defaultClause->v.v ==
                Fortran::parser::OmpDefaultClause::Type::Firstprivate)
         collectSymbols(Fortran::semantics::Symbol::Flag::OmpFirstPrivate);
     }
   }
 }
 
 void DataSharingProcessor::privatize() {
+  for (auto sym : privatizedSymbols) {
+    cloneSymbol(sym);
+    copyFirstPrivateSymbol(sym);
+  }
+}
+
+void DataSharingProcessor::copyLastPrivatize(mlir::Operation *op) {
+  insertLastPrivateCompare(op);
   for (auto sym : privatizedSymbols)
-    privatizeSymbol(sym, &lastPrivIP);
+    copyLastPrivateSymbol(sym, &lastPrivIP);
 }
 
 void DataSharingProcessor::defaultPrivatize() {
-  for (auto sym : defaultSymbols)
+  for (auto sym : defaultSymbols) {
     if (!symbolsInNestedRegions.contains(sym) &&
         !symbolsInParentRegions.contains(sym) &&
-        !privatizedSymbols.contains(sym))
-      privatizeSymbol(sym);
+        !privatizedSymbols.contains(sym)) {
+      cloneSymbol(sym);
+      copyFirstPrivateSymbol(sym);
+    }
+  }
 }
 
 /// The COMMON block is a global structure. \p commonValue is the base address
 /// of the the COMMON block. As the offset from the symbol \p sym, generate the
 /// COMMON block member value (commonValue + offset) for the symbol.
 /// FIXME: Share the code with `instantiateCommon` in ConvertVariable.cpp.
 static mlir::Value
 genCommonBlockMember(Fortran::lower::AbstractConverter &converter,

 /// \param [in]    outerCombined - is this an outer operation - prevents
 ///                                privatization.
 template <typename Op>
 static void
 createBodyOfOp(Op &op, Fortran::lower::AbstractConverter &converter,
                mlir::Location &loc, Fortran::lower::pft::Evaluation &eval,
                const Fortran::parser::OmpClauseList *clauses = nullptr,
                const SmallVector<const Fortran::semantics::Symbol *> &args = {},
-               bool outerCombined = false) {
+               bool outerCombined = false,
+               DataSharingProcessor *dsp = nullptr) {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   // If an argument for the region is provided then create the block with that
   // argument. Also update the symbol's address with the mlir argument value.
   // e.g. For loops the argument is the induction variable. And all further
   // uses of the induction variable should use this mlir value.
   mlir::Operation *storeOp = nullptr;
   if (args.size()) {
     std::size_t loopVarTypeSize = 0;
if constexpr (std::is_same_v<Op, omp::WsLoopOp> ||
     firOpBuilder.create<mlir::omp::TerminatorOp>(loc);
   }
 
   // Reset the insert point to before the terminator.
   resetBeforeTerminator(firOpBuilder, storeOp, block);
 
   // Handle privatization. Do not privatize if this is the outer operation.
   if (clauses && !outerCombined) {
-    DataSharingProcessor dsp(op, converter, *clauses, eval);
-    bool lastPrivateOp = dsp.process();
+    bool lastPrivateOp = false;
+    if (!dsp) {
+      dsp = new DataSharingProcessor(converter, *clauses, eval);
+      lastPrivateOp = dsp->process(op);
+      delete dsp;
+    } else {
+      lastPrivateOp = dsp->processStep2(op);
+    }
     // LastPrivatization, due to introduction of
     // new control flow, changes the insertion point,
     // thus restore it.
     // TODO: Clean up later a bit to avoid this many sets and resets.
     if (lastPrivateOp && !std::is_same_v<Op, omp::SectionOp>)
       resetBeforeTerminator(firOpBuilder, storeOp, block);
   }
 
static void genOMP(Fortran::lower::AbstractConverter &converter,
   llvm::SmallVector<mlir::Value> lowerBound, upperBound, step, linearVars,
       linearStepVars, reductionVars, alignedVars, nontemporalVars;
   mlir::Value scheduleChunkClauseOperand, ifClauseOperand;
   mlir::Attribute scheduleClauseOperand, noWaitClauseOperand,
       orderedClauseOperand, orderClauseOperand;
   mlir::IntegerAttr simdlenClauseOperand, safelenClauseOperand;
   SmallVector<Attribute> reductionDeclSymbols;
   Fortran::lower::StatementContext stmtCtx;
   const auto &loopOpClauseList = std::get<Fortran::parser::OmpClauseList>(
       std::get<Fortran::parser::OmpBeginLoopDirective>(loopConstruct.t).t);
 
   const auto ompDirective =
       std::get<Fortran::parser::OmpLoopDirective>(
           std::get<Fortran::parser::OmpBeginLoopDirective>(loopConstruct.t).t)
           .v;
   if (llvm::omp::OMPD_parallel_do == ompDirective) {
     createCombinedParallelOp<Fortran::parser::OmpBeginLoopDirective>(
         converter, eval,
         std::get<Fortran::parser::OmpBeginLoopDirective>(loopConstruct.t));
   } else if (llvm::omp::OMPD_do != ompDirective &&
              llvm::omp::OMPD_simd != ompDirective) {
     TODO(converter.getCurrentLocation(), "Construct enclosing do loop");
   }
 
+  DataSharingProcessor dsp(converter, loopOpClauseList, eval);
+  dsp.processStep1();
+
   // Collect the loops to collapse.
   auto *doConstructEval = &eval.getFirstNestedEvaluation();
   if (doConstructEval->getIf<Fortran::parser::DoConstruct>()
           ->IsDoConcurrent()) {
     TODO(converter.getCurrentLocation(),
          "Do Concurrent in Worksharing loop construct");
   }
 
if (llvm::omp::OMPD_simd == ompDirective) {
     TypeRange resultType;
     auto simdLoopOp = firOpBuilder.create<mlir::omp::SimdLoopOp>(
         currentLocation, resultType, lowerBound, upperBound, step, alignedVars,
         nullptr, ifClauseOperand, nontemporalVars,
         orderClauseOperand.dyn_cast_or_null<omp::ClauseOrderKindAttr>(),
         simdlenClauseOperand, safelenClauseOperand,
         /*inclusive=*/firOpBuilder.getUnitAttr());
     createBodyOfOp<omp::SimdLoopOp>(simdLoopOp, converter, currentLocation,
-                                    eval, &loopOpClauseList, iv);
+                                    eval, &loopOpClauseList, iv,
+                                    /*outer=*/false, &dsp);
     return;
   }
 
   // FIXME: Add support for following clauses:
   // 1. linear
   // 2. order
   auto wsLoopOp = firOpBuilder.create<mlir::omp::WsLoopOp>(
       currentLocation, lowerBound, upperBound, step, linearVars, linearStepVars,
if (const auto &endClauseList =
     const auto &clauseList =
         std::get<Fortran::parser::OmpClauseList>((*endClauseList).t);
     for (const Fortran::parser::OmpClause &clause : clauseList.v)
       if (std::get_if<Fortran::parser::OmpClause::Nowait>(&clause.u))
         wsLoopOp.setNowaitAttr(firOpBuilder.getUnitAttr());
   }
 
   createBodyOfOp<omp::WsLoopOp>(wsLoopOp, converter, currentLocation, eval,
-                                &loopOpClauseList, iv);
+                                &loopOpClauseList, iv, /*outer=*/false, &dsp);
 }
 
 static void
 genOMP(Fortran::lower::AbstractConverter &converter,
        Fortran::lower::pft::Evaluation &eval,
        const Fortran::parser::OpenMPCriticalConstruct &criticalConstruct) {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   mlir::Location currentLocation = converter.getCurrentLocation();

flang/test/Lower/OpenMP/parallel-lastprivate-clause-scalar.f90

 !$OMP END DO
 !$OMP END PARALLEL
 print *, arg1, arg2
 end subroutine
 
 !CHECK: func.func @_QPfirstpriv_lastpriv_int2(%[[ARG1:.*]]: !fir.ref<i32> {fir.bindc_name = "arg1"}) {
 !CHECK: omp.parallel  {
 ! Firstprivate update
-!CHECK-NEXT: %[[CLONE1:.*]] = fir.alloca i32 {bindc_name = "arg1"
+!CHECK: %[[CLONE1:.*]] = fir.alloca i32 {bindc_name = "arg1"
 !CHECK-NEXT: %[[FPV_LD:.*]] = fir.load %[[ARG1]] : !fir.ref<i32>
 !CHECK-NEXT: fir.store %[[FPV_LD]] to %[[CLONE1]] : !fir.ref<i32>
 !CHECK-NEXT: omp.barrier
 !CHECK: omp.wsloop for (%[[INDX_WS:.*]]) : {{.*}} {
 ! Testing last iteration check
 !CHECK: %[[IV_CMP1:.*]] = arith.cmpi eq, %[[INDX_WS]]
 !CHECK-NEXT: fir.if %[[IV_CMP1]] {
 ! Testing lastprivate val update

flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90

 ! This test checks a few bug fixes in the PRIVATE clause lowering
 
 ! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
 
 ! CHECK-LABEL: multiple_private_fix
 ! CHECK:         %[[VAL_0:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFmultiple_private_fixEi"}
 ! CHECK:         %[[VAL_1:.*]] = fir.alloca i32 {bindc_name = "j", uniq_name = "_QFmultiple_private_fixEj"}
 ! CHECK:         %[[VAL_2:.*]] = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFmultiple_private_fixEx"}
 ! CHECK:         omp.parallel {
 ! CHECK:           %[[PRIV_J:.*]] = fir.alloca i32 {bindc_name = "j", pinned
+! CHECK:           %[[PRIV_I:.*]] = fir.alloca i32 {adapt.valuebyref, pinned
 ! CHECK:           %[[PRIV_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned
-! CHECK:           %[[PRIV_I:.*]] = fir.alloca i32 {{{.*}}, pinned
 ! CHECK:           %[[ONE:.*]] = arith.constant 1 : i32
 ! CHECK:           %[[VAL_3:.*]] = fir.load %[[VAL_4:.*]] : !fir.ref<i32>
 ! CHECK:           %[[VAL_5:.*]] = arith.constant 1 : i32
 ! CHECK:           omp.wsloop for (%[[VAL_6:.*]]) : i32 = (%[[ONE]]) to (%[[VAL_3]]) inclusive step (%[[VAL_5]]) {
 ! CHECK:             fir.store %[[VAL_6]] to %[[PRIV_I]] : !fir.ref<i32>
 ! CHECK:             %[[VAL_7:.*]] = arith.constant 1 : i32
 ! CHECK:             %[[VAL_8:.*]] = fir.convert %[[VAL_7]] : (i32) -> index
 ! CHECK:             %[[VAL_9:.*]] = fir.load %[[VAL_4]] : !fir.ref<i32>

flang/test/Lower/OpenMP/parallel-wsloop-firstpriv.f90

 ! This test checks lowering of OpenMP parallel DO, with the loop bound being
 ! a firstprivate variable
 
 ! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
 
 ! CHECK: func @_QPomp_do_firstprivate(%[[ARG0:.*]]: !fir.ref<i32> {fir.bindc_name = "a"}) 
 subroutine omp_do_firstprivate(a)
   integer::a
   integer::n
   n = a+1
   !$omp parallel do firstprivate(a)
   ! CHECK:  omp.parallel {
+  ! CHECK-NEXT: %[[REF:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
   ! CHECK-NEXT: %[[CLONE:.*]] = fir.alloca i32 {bindc_name = "a", pinned
   ! CHECK-NEXT: %[[LD:.*]] = fir.load %[[ARG0]] : !fir.ref<i32>
   ! CHECK-NEXT: fir.store %[[LD]] to %[[CLONE]] : !fir.ref<i32>
-  ! CHECK-NEXT: %[[REF:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
   ! CHECK: %[[LB:.*]] = arith.constant 1 : i32
   ! CHECK-NEXT: %[[UB:.*]] = fir.load %[[CLONE]] : !fir.ref<i32>
   ! CHECK-NEXT: %[[STEP:.*]] = arith.constant 1 : i32
   ! CHECK-NEXT: omp.wsloop   for  (%[[ARG1:.*]]) : i32 = (%[[LB]]) to (%[[UB]]) inclusive step (%[[STEP]])
   ! CHECK-NEXT: fir.store %[[ARG1]] to %[[REF]] : !fir.ref<i32>
   ! CHECK-NEXT: fir.call @_QPfoo(%[[REF]], %[[CLONE]]) {{.*}}: (!fir.ref<i32>, !fir.ref<i32>) -> ()
   ! CHECK-NEXT: omp.yield
     do i=1, a
       call foo(i, a)
     end do
   !$omp end parallel do
   !CHECK: fir.call @_QPbar(%[[ARG0]]) {{.*}}: (!fir.ref<i32>) -> ()
   call bar(a)
 end subroutine omp_do_firstprivate
 
 ! CHECK: func @_QPomp_do_firstprivate2(%[[ARG0:.*]]: !fir.ref<i32> {fir.bindc_name = "a"}, %[[ARG1:.*]]: !fir.ref<i32> {fir.bindc_name = "n"}) 
 subroutine omp_do_firstprivate2(a, n)
   integer::a
   integer::n
   n = a+1
   !$omp parallel do firstprivate(a, n)
   ! CHECK:  omp.parallel {
+  ! CHECK-NEXT: %[[REF:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
   ! CHECK-NEXT: %[[CLONE:.*]] = fir.alloca i32 {bindc_name = "a", pinned
   ! CHECK-NEXT: %[[LD:.*]] = fir.load %[[ARG0]] : !fir.ref<i32>
   ! CHECK-NEXT: fir.store %[[LD]] to %[[CLONE]] : !fir.ref<i32>
   ! CHECK-NEXT: %[[CLONE1:.*]] = fir.alloca i32 {bindc_name = "n", pinned
   ! CHECK-NEXT: %[[LD1:.*]] = fir.load %[[ARG1]] : !fir.ref<i32>
   ! CHECK-NEXT: fir.store %[[LD1]] to %[[CLONE1]] : !fir.ref<i32>
-  ! CHECK-NEXT: %[[REF:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
 
 
   ! CHECK: %[[LB:.*]] = fir.load %[[CLONE]] : !fir.ref<i32>
   ! CHECK-NEXT: %[[UB:.*]] = fir.load %[[CLONE1]] : !fir.ref<i32>
   ! CHECK-NEXT: %[[STEP:.*]] = arith.constant 1 : i32
   ! CHECK-NEXT: omp.wsloop   for  (%[[ARG2:.*]]) : i32 = (%[[LB]]) to (%[[UB]]) inclusive step (%[[STEP]])
   ! CHECK-NEXT: fir.store %[[ARG2]] to %[[REF]] : !fir.ref<i32>
   ! CHECK-NEXT: fir.call @_QPfoo(%[[REF]], %[[CLONE]]) {{.*}}: (!fir.ref<i32>, !fir.ref<i32>) -> ()
   ! CHECK-NEXT: omp.yield
     do i= a, n
       call foo(i, a)
     end do
   !$omp end parallel do
   !CHECK: fir.call @_QPbar(%[[ARG1]]) {{.*}}: (!fir.ref<i32>) -> ()
   call bar(n)
 end subroutine omp_do_firstprivate2

flang/test/Lower/OpenMP/parallel-wsloop.f90

logical :: cond
   !$OMP END PARALLEL
 end subroutine parallel_do_private
 
 ! CHECK-LABEL:   func.func @_QPparallel_do_private(
 ! CHECK-SAME:                                      %[[VAL_0:.*]]: !fir.ref<!fir.logical<4>> {fir.bindc_name = "cond"},
 ! CHECK-SAME:                                      %[[VAL_1:.*]]: !fir.ref<i32> {fir.bindc_name = "nt"}) {
 ! CHECK:           %[[I_ADDR:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFparallel_do_privateEi"}
 ! CHECK:           omp.parallel   {
+! CHECK:             %[[I_PRIV_ADDR:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
 ! CHECK:             %[[COND_ADDR:.*]] = fir.alloca !fir.logical<4> {bindc_name = "cond", pinned, uniq_name = "_QFparallel_do_privateEcond"}
 ! CHECK:             %[[NT_ADDR:.*]] = fir.alloca i32 {bindc_name = "nt", pinned, uniq_name = "_QFparallel_do_privateEnt"}
 ! CHECK:             %[[NT:.*]] = fir.load %[[VAL_1]] : !fir.ref<i32>
 ! CHECK:             fir.store %[[NT]] to %[[NT_ADDR]] : !fir.ref<i32>
-! CHECK:             %[[I_PRIV_ADDR:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
 ! CHECK:             %[[VAL_7:.*]] = arith.constant 1 : i32
 ! CHECK:             %[[VAL_8:.*]] = arith.constant 9 : i32
 ! CHECK:             %[[VAL_9:.*]] = arith.constant 1 : i32
 ! CHECK:             omp.wsloop   for  (%[[I:.*]]) : i32 = (%[[VAL_7]]) to (%[[VAL_8]]) inclusive step (%[[VAL_9]]) {
 ! CHECK:               fir.store %[[I]] to %[[I_PRIV_ADDR]] : !fir.ref<i32>
 ! CHECK:               fir.call @_QPfoo(%[[I_PRIV_ADDR]], %[[COND_ADDR]], %[[NT_ADDR]]) {{.*}}: (!fir.ref<i32>, !fir.ref<!fir.logical<4>>, !fir.ref<i32>) -> ()
 ! CHECK:               omp.yield
 ! CHECK:             }
subroutine omp_parallel_do_multiple_firstprivate(a, b)
   !$OMP END PARALLEL
 end subroutine omp_parallel_do_multiple_firstprivate
 
 ! CHECK-LABEL:   func.func @_QPomp_parallel_do_multiple_firstprivate(
 ! CHECK-SAME:                                                        %[[A_ADDR:.*]]: !fir.ref<i32> {fir.bindc_name = "a"},
 ! CHECK-SAME:                                                        %[[B_ADDR:.*]]: !fir.ref<i32> {fir.bindc_name = "b"}) {
 ! CHECK:           %[[I_ADDR:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFomp_parallel_do_multiple_firstprivateEi"}
 ! CHECK:           omp.parallel   {
+! CHECK:             %[[I_PRIV_ADDR:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
 ! CHECK:             %[[A_PRIV_ADDR:.*]] = fir.alloca i32 {bindc_name = "a", pinned, uniq_name = "_QFomp_parallel_do_multiple_firstprivateEa"}
 ! CHECK:             %[[A:.*]] = fir.load %[[A_ADDR]] : !fir.ref<i32>
 ! CHECK:             fir.store %[[A]] to %[[A_PRIV_ADDR]] : !fir.ref<i32>
 ! CHECK:             %[[B_PRIV_ADDR:.*]] = fir.alloca i32 {bindc_name = "b", pinned, uniq_name = "_QFomp_parallel_do_multiple_firstprivateEb"}
 ! CHECK:             %[[B:.*]] = fir.load %[[B_ADDR]] : !fir.ref<i32>
 ! CHECK:             fir.store %[[B]] to %[[B_PRIV_ADDR]] : !fir.ref<i32>
-! CHECK:             %[[I_PRIV_ADDR:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
 ! CHECK:             %[[VAL_8:.*]] = arith.constant 1 : i32
 ! CHECK:             %[[VAL_9:.*]] = arith.constant 10 : i32
 ! CHECK:             %[[VAL_10:.*]] = arith.constant 1 : i32
 ! CHECK:             omp.wsloop   for  (%[[I:.*]]) : i32 = (%[[VAL_8]]) to (%[[VAL_9]]) inclusive step (%[[VAL_10]]) {
 ! CHECK:               fir.store %[[I]] to %[[I_PRIV_ADDR]] : !fir.ref<i32>
 ! CHECK:               fir.call @_QPbar(%[[I_PRIV_ADDR]], %[[A_PRIV_ADDR]]) {{.*}}: (!fir.ref<i32>, !fir.ref<i32>) -> ()
 ! CHECK:               omp.yield
 ! CHECK:             }
 ! CHECK:             omp.terminator
 ! CHECK:           }
 ! CHECK:           return
 ! CHECK:         }

flang/test/Lower/OpenMP/single.f90

 
 !===============================================================================
 ! Single construct with private/firstprivate
 !===============================================================================
 
 ! CHECK-LABEL: func.func @_QPsingle_privatization(
 ! CHECK-SAME:                                     %[[VAL_0:.*]]: !fir.ref<f32> {fir.bindc_name = "x"},
 ! CHECK-SAME:                                     %[[VAL_1:.*]]: !fir.ref<f64> {fir.bindc_name = "y"}) {
-! CHECK:         %[[VAL_2:.*]] = fir.alloca f32 {bindc_name = "x", pinned, uniq_name = "_QFsingle_privatizationEx"}
-! CHECK:         %[[VAL_3:.*]] = fir.alloca f64 {bindc_name = "y", pinned, uniq_name = "_QFsingle_privatizationEy"}
-! CHECK:         omp.single   {
+! CHECK:         omp.single   {
+! CHECK:           %[[VAL_2:.*]] = fir.alloca f32 {bindc_name = "x", pinned, uniq_name = "_QFsingle_privatizationEx"}
+! CHECK:           %[[VAL_3:.*]] = fir.alloca f64 {bindc_name = "y", pinned, uniq_name = "_QFsingle_privatizationEy"}
 ! CHECK:           %[[VAL_4:.*]] = fir.load %[[VAL_1]] : !fir.ref<f64>
 ! CHECK:           fir.store %[[VAL_4]] to %[[VAL_3]] : !fir.ref<f64>
 ! CHECK:           fir.call @_QPbar(%[[VAL_2]], %[[VAL_3]]) {{.*}}: (!fir.ref<f32>, !fir.ref<f64>) -> ()
 ! CHECK:           omp.terminator
 ! CHECK:         }
 ! CHECK:         return
 ! CHECK:       }
 
 subroutine single_privatization(x, y)
   real :: x
   real(8) :: y
 
   !$omp single private(x) firstprivate(y)
   call bar(x, y)
   !$omp end single
 end subroutine
 
 ! CHECK-LABEL: func.func @_QPsingle_privatization2(
 ! CHECK-SAME:                                      %[[VAL_0:.*]]: !fir.ref<f32> {fir.bindc_name = "x"},
 ! CHECK-SAME:                                      %[[VAL_1:.*]]: !fir.ref<f64> {fir.bindc_name = "y"}) {
 ! CHECK:         omp.parallel   {
-! CHECK:           %[[VAL_2:.*]] = fir.alloca f32 {bindc_name = "x", pinned, uniq_name = "_QFsingle_privatization2Ex"}
-! CHECK:           %[[VAL_3:.*]] = fir.alloca f64 {bindc_name = "y", pinned, uniq_name = "_QFsingle_privatization2Ey"}
-! CHECK:           omp.single   {
+! CHECK:           omp.single   {
+! CHECK:             %[[VAL_2:.*]] = fir.alloca f32 {bindc_name = "x", pinned, uniq_name = "_QFsingle_privatization2Ex"}
+! CHECK:             %[[VAL_3:.*]] = fir.alloca f64 {bindc_name = "y", pinned, uniq_name = "_QFsingle_privatization2Ey"}
 ! CHECK:             %[[VAL_4:.*]] = fir.load %[[VAL_1]] : !fir.ref<f64>
 ! CHECK:             fir.store %[[VAL_4]] to %[[VAL_3]] : !fir.ref<f64>
 ! CHECK:             fir.call @_QPbar(%[[VAL_2]], %[[VAL_3]]) {{.*}}: (!fir.ref<f32>, !fir.ref<f64>) -> ()
 ! CHECK:             omp.terminator
 ! CHECK:           }
 ! CHECK:           omp.terminator
 ! CHECK:         }
 ! CHECK:         return