[OpenMP] Honor barrier and last iteration flags in loop lowering by nicebert · Pull Request #224040 · llvm/llvm-project

nicebert · 2026-09-16T14:59:25Z

Emitting loop-free kernels for no-loop target regions in Clang requires
the shared OpenMP lowering to honor flags that reach it today and are
then discarded.

applyWorkshareLoop takes a NeedsBarrier flag, but the device path drops
it, so a worksharing loop without nowait emits no barrier at its exit.
The device path also never sets the canonical loop's last iteration
variable, which the linear clause finalization reads. Forward the flag
to applyWorkshareLoopTarget and compute the last iteration in the loop
body, mirroring how the host runtime reports it.

Auditing the surrounding lowering for the same class of problem turned
up one more. The barrier that follows privatization is emitted as part
of the firstprivate copy region, so a construct with lastprivate and no
firstprivate never gets one. Emit it independently of the copy region.
Flang skips it for taskloop, where the write-back already happens after
the reads.

Split out of #205325.

Emitting loop-free kernels for no-loop target regions in Clang requires the shared OpenMP lowering to honor flags that reach it today and are then discarded. applyWorkshareLoop takes a NeedsBarrier flag, but the device path drops it, so a worksharing loop without nowait emits no barrier at its exit. The device path also never sets the canonical loop's last iteration variable, which the linear clause finalization reads. Forward the flag to applyWorkshareLoopTarget and compute the last iteration in the loop body, mirroring how the host runtime reports it. Auditing the surrounding lowering for the same class of problem turned up one more. The barrier that follows privatization is emitted as part of the firstprivate copy region, so a construct with lastprivate and no firstprivate never gets one. Emit it independently of the copy region. Flang skips it for taskloop, where the write-back already happens after the reads.

llvmorg-github-actions · 2026-09-16T15:00:03Z

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

@llvm/pr-subscribers-flang-openmp

Author: Nicole Aschenbrenner (nicebert)

Changes

Emitting loop-free kernels for no-loop target regions in Clang requires
the shared OpenMP lowering to honor flags that reach it today and are
then discarded.

applyWorkshareLoop takes a NeedsBarrier flag, but the device path drops
it, so a worksharing loop without nowait emits no barrier at its exit.
The device path also never sets the canonical loop's last iteration
variable, which the linear clause finalization reads. Forward the flag
to applyWorkshareLoopTarget and compute the last iteration in the loop
body, mirroring how the host runtime reports it.

Auditing the surrounding lowering for the same class of problem turned
up one more. The barrier that follows privatization is emitted as part
of the firstprivate copy region, so a construct with lastprivate and no
firstprivate never gets one. Emit it independently of the copy region.
Flang skips it for taskloop, where the write-back already happens after
the reads.

Split out of #205325.

Patch is 22.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/224040.diff

7 Files Affected:

(modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+7)
(added) flang/test/Integration/OpenMP/privatization-barrier.f90 (+38)
(modified) flang/test/Lower/OpenMP/taskloop.f90 (+15)
(modified) llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h (+10-6)
(modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+38-4)
(modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+53-43)
(modified) mlir/test/Target/LLVMIR/omptarget-wsloop.mlir (+40)

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 8d0d191058cfb..3e63c068e1cdb 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -372,6 +372,13 @@ bool DataSharingProcessor::needBarrier() {
   // initialization of firstprivate variables and post-update of lastprivate
   // variables.
   // Emit implicit barrier for linear clause in the OpenMPIRBuilder.
+  // Skip for taskloop: the write-back only happens after the reads are done.
+
+  const auto *ompEval = eval.getIf<parser::OpenMPConstruct>();
+  if (ompEval && llvm::omp::allTaskloopSet.test(
+                     parser::omp::GetOmpDirectiveName(*ompEval).v))
+    return false;
+
   for (const semantics::Symbol *sym : allPrivatizedSymbols) {
     if (sym->test(semantics::Symbol::Flag::OmpLastPrivate) &&
         (sym->test(semantics::Symbol::Flag::OmpFirstPrivate) ||
diff --git a/flang/test/Integration/OpenMP/privatization-barrier.f90 b/flang/test/Integration/OpenMP/privatization-barrier.f90
new file mode 100644
index 0000000000000..06ab26df2521e
--- /dev/null
+++ b/flang/test/Integration/OpenMP/privatization-barrier.f90
@@ -0,0 +1,38 @@
+!===----------------------------------------------------------------------===!
+! This directory can be used to add Integration tests involving multiple
+! stages of the compiler (for eg. from Fortran to LLVM IR). It should not
+! contain executable tests. We should only add tests here sparingly and only
+! if there is no other way to test. Repeat this message in each test that is
+! added to this directory and sub-directories.
+!===----------------------------------------------------------------------===!
+
+! RUN: %flang_fc1 -fopenmp -emit-llvm %s -o - | FileCheck %s
+! RUN: %if amdgpu-registered-target %{ %flang_fc1 -triple amdgcn-amd-amdhsa -emit-llvm -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s %}
+
+subroutine lastprivate_allocatable_barrier_host
+  integer, allocatable :: a
+  integer :: i
+  !$omp parallel do lastprivate(a)
+  do i = 1, 10
+    a = i
+  end do
+  !$omp end parallel do
+end subroutine
+
+subroutine lastprivate_allocatable_barrier_device
+  integer, allocatable :: a
+  integer :: i
+  allocate(a)
+  !$omp target parallel do lastprivate(a)
+  do i = 1, 10
+    a = i
+  end do
+  !$omp end target parallel do
+end subroutine
+
+! CHECK-LABEL: define internal void @{{.*}}lastprivate_allocatable_barrier_{{(host|device)}}
+! CHECK:         call void @__kmpc_barrier
+! CHECK-NEXT:    br label %omp.wsloop.region
+! CHECK:         call void @__kmpc_barrier
+! CHECK-NEXT:    br label %omp_loop.after
+! CHECK-LABEL: define{{.*}}void @{{.*}}lastprivate_allocatable_barrier_device
diff --git a/flang/test/Lower/OpenMP/taskloop.f90 b/flang/test/Lower/OpenMP/taskloop.f90
index 94e4e2947fa71..c9ce984db7e8e 100644
--- a/flang/test/Lower/OpenMP/taskloop.f90
+++ b/flang/test/Lower/OpenMP/taskloop.f90
@@ -2,6 +2,9 @@
 ! RUN: bbc -emit-hlfir %openmp_flags -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s
 ! RUN: %flang_fc1 -emit-hlfir %openmp_flags -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s
 
+! CHECK-LABEL:  omp.private {type = firstprivate}
+! CHECK-SAME:     @[[FIRST_LAST_PRIVATE_X:.*]] : i32
+
 ! CHECK-LABEL:  omp.private
 ! CHECK-SAME:       {type = private} @[[LAST_PRIVATE_I:.*]] : i32
 
@@ -260,3 +263,15 @@ subroutine omp_taskloop_lastprivate()
    ! CHECK:  omp.terminator
    !$omp end taskloop
 end subroutine omp_taskloop_lastprivate
+
+! CHECK-LABEL:  func @_QPomp_taskloop_first_and_lastprivate()
+subroutine omp_taskloop_first_and_lastprivate()
+   integer x
+   x = 0
+   ! CHECK:  omp.taskloop.context private(@[[FIRST_LAST_PRIVATE_X]] {{.*}}) {
+   !$omp taskloop firstprivate(x) lastprivate(x)
+   do i = 1, 100
+      x = x + 1
+   end do
+   !$omp end taskloop
+end subroutine omp_taskloop_first_and_lastprivate
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index a13832d69b6a0..90a91e98b4863 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -1177,15 +1177,19 @@ class OpenMPIRBuilder {
   /// \param CLI      A descriptor of the canonical loop to workshare.
   /// \param AllocaIP An insertion point for Alloca instructions usable in the
   ///                 preheader of the loop.
+  /// \param NeedsBarrier Indicates whether a barrier must be inserted after
+  ///                     the loop.
   /// \param LoopType Information about type of loop worksharing.
   ///                 It corresponds to type of loop workshare OpenMP pragma.
   /// \param NoLoop   If true, no-loop code is generated.
+  /// \param NeedsLastIter  If true, the last iteration variable is emitted.
   ///
   /// \returns Point where to insert code after the workshare construct.
-  InsertPointTy applyWorkshareLoopTarget(DebugLoc DL, CanonicalLoopInfo *CLI,
-                                         InsertPointTy AllocaIP,
-                                         omp::WorksharingLoopType LoopType,
-                                         bool NoLoop);
+  InsertPointOrErrorTy
+  applyWorkshareLoopTarget(DebugLoc DL, CanonicalLoopInfo *CLI,
+                           InsertPointTy AllocaIP, bool NeedsBarrier,
+                           omp::WorksharingLoopType LoopType, bool NoLoop,
+                           bool NeedsLastIter);
 
   /// Modifies the canonical loop to be a statically-scheduled workshare loop.
   ///
@@ -1343,8 +1347,8 @@ class OpenMPIRBuilder {
   /// \param NoLoop If true, no-loop code is generated.
   /// \param HasDistSchedule Defines if the clause being lowered is
   /// dist_schedule as this is handled slightly differently
-  ///
   /// \param DistScheduleChunkSize The chunk size for dist_schedule loop
+  /// \param NeedsLastIter  If true, the last iteration variable is emitted.
   ///
   /// \returns Point where to insert code after the workshare construct.
   LLVM_ABI InsertPointOrErrorTy applyWorkshareLoop(
@@ -1357,7 +1361,7 @@ class OpenMPIRBuilder {
       omp::WorksharingLoopType LoopType =
           omp::WorksharingLoopType::ForStaticLoop,
       bool NoLoop = false, bool HasDistSchedule = false,
-      Value *DistScheduleChunkSize = nullptr);
+      Value *DistScheduleChunkSize = nullptr, bool NeedsLastIter = false);
 
   /// Tile a loop nest.
   ///
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 0747b943d1ba4..8741ccba37d08 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -6612,11 +6612,32 @@ static void workshareLoopTargetCallback(
   CLI->invalidate();
 }
 
-OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::applyWorkshareLoopTarget(
+OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::applyWorkshareLoopTarget(
     DebugLoc DL, CanonicalLoopInfo *CLI, InsertPointTy AllocaIP,
-    WorksharingLoopType LoopType, bool NoLoop) {
+    bool NeedsBarrier, WorksharingLoopType LoopType, bool NoLoop,
+    bool NeedsLastIter) {
   uint32_t SrcLocStrSize;
   Constant *SrcLocStr = getOrCreateSrcLocStr(DL, SrcLocStrSize);
+
+  // Mirrors host runtime reporting of last iteration by in-body computation.
+  if (NeedsLastIter) {
+    Type *I32Type = Type::getInt32Ty(M.getContext());
+    Builder.restoreIP(AllocaIP);
+    AllocaInst *PLastIter =
+        Builder.CreateAlloca(I32Type, nullptr, "p.lastiter");
+    Builder.CreateStore(ConstantInt::get(I32Type, 0), PLastIter);
+    CLI->setLastIter(PLastIter);
+
+    Builder.SetInsertPoint(CLI->getBody(),
+                           CLI->getBody()->getFirstInsertionPt());
+    Value *TripCount = CLI->getTripCount();
+    Value *LastIter =
+        Builder.CreateSub(TripCount, ConstantInt::get(TripCount->getType(), 1));
+    Value *IsLast =
+        Builder.CreateICmpEQ(CLI->getIndVar(), LastIter, "omp.is_last_iter");
+    Builder.CreateStore(Builder.CreateZExt(IsLast, I32Type), PLastIter);
+  }
+
   IdentFlag Flag = IdentFlag(0);
   switch (LoopType) {
   case WorksharingLoopType::ForStaticLoop:
@@ -6713,6 +6734,18 @@ OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::applyWorkshareLoopTarget(
                                 LoopType, NoLoop);
   };
   addOutlineInfo(std::move(OI));
+
+  if (NeedsBarrier) {
+    Builder.SetInsertPoint(CLI->getExit(),
+                           CLI->getExit()->getTerminator()->getIterator());
+    InsertPointOrErrorTy BarrierIP =
+        createBarrier(LocationDescription(Builder.saveIP(), DL),
+                      omp::Directive::OMPD_for, /* ForceSimpleCall */ false,
+                      /* CheckCancelFlag*/ false);
+    if (!BarrierIP)
+      return BarrierIP.takeError();
+  }
+
   return CLI->getAfterIP();
 }
 
@@ -6722,9 +6755,10 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::applyWorkshareLoop(
     bool HasSimdModifier, bool HasMonotonicModifier,
     bool HasNonmonotonicModifier, bool HasOrderedClause,
     WorksharingLoopType LoopType, bool NoLoop, bool HasDistSchedule,
-    Value *DistScheduleChunkSize) {
+    Value *DistScheduleChunkSize, bool NeedsLastIter) {
   if (Config.isTargetDevice())
-    return applyWorkshareLoopTarget(DL, CLI, AllocaIP, LoopType, NoLoop);
+    return applyWorkshareLoopTarget(DL, CLI, AllocaIP, NeedsBarrier, LoopType,
+                                    NoLoop, NeedsLastIter);
   OMPScheduleType EffectiveScheduleType = computeOpenMPScheduleType(
       SchedKind, ChunkSize, HasSimdModifier, HasMonotonicModifier,
       HasNonmonotonicModifier, HasOrderedClause, DistScheduleChunkSize);
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index e5e3e0cc6d944..f6a4b4c5d7f3f 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -2083,13 +2083,27 @@ static bool opIsInSingleThread(mlir::Operation *op) {
   return false;
 }
 
-static LogicalResult copyFirstPrivateVars(
-    mlir::Operation *op, llvm::IRBuilderBase &builder,
-    LLVM::ModuleTranslation &moduleTranslation,
-    SmallVectorImpl<llvm::Value *> &moldVars,
-    ArrayRef<llvm::Value *> llvmPrivateVars,
-    SmallVectorImpl<omp::PrivateClauseOp> &privateDecls, bool insertBarrier,
-    llvm::DenseMap<Value, Value> *mappedPrivateVars = nullptr) {
+static LogicalResult
+emitPrivatizationBarrier(mlir::Operation *op, llvm::IRBuilderBase &builder,
+                         LLVM::ModuleTranslation &moduleTranslation,
+                         bool insertBarrier) {
+  if (!insertBarrier || opIsInSingleThread(op))
+    return success();
+
+  llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
+  llvm::OpenMPIRBuilder::InsertPointOrErrorTy res =
+      ompBuilder->createBarrier(builder, llvm::omp::OMPD_barrier);
+  return handleError(res, *op);
+}
+
+static LogicalResult
+completePrivateVars(mlir::Operation *op, llvm::IRBuilderBase &builder,
+                    LLVM::ModuleTranslation &moduleTranslation,
+                    SmallVectorImpl<llvm::Value *> &moldVars,
+                    ArrayRef<llvm::Value *> llvmPrivateVars,
+                    SmallVectorImpl<omp::PrivateClauseOp> &privateDecls,
+                    bool insertBarrier,
+                    llvm::DenseMap<Value, Value> *mappedPrivateVars = nullptr) {
   // Apply copy region for firstprivate.
   bool needsFirstprivate =
       llvm::any_of(privateDecls, [](omp::PrivateClauseOp &privOp) {
@@ -2098,7 +2112,8 @@ static LogicalResult copyFirstPrivateVars(
       });
 
   if (!needsFirstprivate)
-    return success();
+    return emitPrivatizationBarrier(op, builder, moduleTranslation,
+                                    insertBarrier);
 
   llvm::BasicBlock *copyBlock =
       splitBB(builder, /*CreateBranch=*/true, "omp.private.copy");
@@ -2137,24 +2152,18 @@ static LogicalResult copyFirstPrivateVars(
     moduleTranslation.forgetMapping(copyRegion);
   }
 
-  if (insertBarrier && !opIsInSingleThread(op)) {
-    llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
-    llvm::OpenMPIRBuilder::InsertPointOrErrorTy res =
-        ompBuilder->createBarrier(builder, llvm::omp::OMPD_barrier);
-    if (failed(handleError(res, *op)))
-      return failure();
-  }
-
-  return success();
+  return emitPrivatizationBarrier(op, builder, moduleTranslation,
+                                  insertBarrier);
 }
 
-static LogicalResult copyFirstPrivateVars(
-    mlir::Operation *op, llvm::IRBuilderBase &builder,
-    LLVM::ModuleTranslation &moduleTranslation,
-    SmallVectorImpl<mlir::Value> &mlirPrivateVars,
-    ArrayRef<llvm::Value *> llvmPrivateVars,
-    SmallVectorImpl<omp::PrivateClauseOp> &privateDecls, bool insertBarrier,
-    llvm::DenseMap<Value, Value> *mappedPrivateVars = nullptr) {
+static LogicalResult
+completePrivateVars(mlir::Operation *op, llvm::IRBuilderBase &builder,
+                    LLVM::ModuleTranslation &moduleTranslation,
+                    SmallVectorImpl<mlir::Value> &mlirPrivateVars,
+                    ArrayRef<llvm::Value *> llvmPrivateVars,
+                    SmallVectorImpl<omp::PrivateClauseOp> &privateDecls,
+                    bool insertBarrier,
+                    llvm::DenseMap<Value, Value> *mappedPrivateVars = nullptr) {
   llvm::SmallVector<llvm::Value *> moldVars(mlirPrivateVars.size());
   llvm::transform(mlirPrivateVars, moldVars.begin(), [&](mlir::Value mlirVar) {
     // map copyRegion rhs arg
@@ -2163,9 +2172,9 @@ static LogicalResult copyFirstPrivateVars(
     assert(moldVar);
     return moldVar;
   });
-  return copyFirstPrivateVars(op, builder, moduleTranslation, moldVars,
-                              llvmPrivateVars, privateDecls, insertBarrier,
-                              mappedPrivateVars);
+  return completePrivateVars(op, builder, moduleTranslation, moldVars,
+                             llvmPrivateVars, privateDecls, insertBarrier,
+                             mappedPrivateVars);
 }
 
 template <typename T>
@@ -2419,7 +2428,7 @@ convertOmpScope(omp::ScopeOp &scopeOp, llvm::IRBuilderBase &builder,
             .failed())
       return llvm::make_error<PreviouslyReportedError>();
 
-    if (failed(copyFirstPrivateVars(
+    if (failed(completePrivateVars(
             scopeOp, builder, moduleTranslation, privateVarsInfo.mlirVars,
             privateVarsInfo.llvmVars, privateVarsInfo.privatizers,
             scopeOp.getPrivateNeedsBarrier())))
@@ -3340,7 +3349,7 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
 
   // firstprivate copy region
   setInsertPointForPossiblyEmptyBlock(builder, copyBlock);
-  if (failed(copyFirstPrivateVars(
+  if (failed(completePrivateVars(
           taskOp, builder, moduleTranslation, privateVarsInfo.mlirVars,
           taskStructMgr.getLLVMPrivateVarGEPs(), privateVarsInfo.privatizers,
           taskOp.getPrivateNeedsBarrier())))
@@ -3811,7 +3820,7 @@ convertOmpTaskloopContextOp(omp::TaskloopContextOp contextOp,
 
   // firstprivate copy region
   setInsertPointForPossiblyEmptyBlock(builder, copyBlock);
-  if (failed(copyFirstPrivateVars(
+  if (failed(completePrivateVars(
           contextOp, builder, moduleTranslation, privateVarsInfo.mlirVars,
           taskStructMgr.getLLVMPrivateVarGEPs(), privateVarsInfo.privatizers,
           contextOp.getPrivateNeedsBarrier())))
@@ -4110,10 +4119,10 @@ convertOmpTaskloopContextOp(omp::TaskloopContextOp contextOp,
       // through a stack allocated structure.
     }
 
-    if (failed(copyFirstPrivateVars(contextOp.getOperation(), builder,
-                                    moduleTranslation, srcGEPs, destGEPs,
-                                    privateVarsInfo.privatizers,
-                                    contextOp.getPrivateNeedsBarrier())))
+    if (failed(completePrivateVars(contextOp.getOperation(), builder,
+                                   moduleTranslation, srcGEPs, destGEPs,
+                                   privateVarsInfo.privatizers,
+                                   contextOp.getPrivateNeedsBarrier())))
       return llvm::make_error<PreviouslyReportedError>();
 
     return builder.saveIP();
@@ -4707,7 +4716,7 @@ convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase &builder,
           .failed())
     return failure();
 
-  if (failed(copyFirstPrivateVars(
+  if (failed(completePrivateVars(
           wsloopOp, builder, moduleTranslation, privateVarsInfo.mlirVars,
           privateVarsInfo.llvmVars, privateVarsInfo.privatizers,
           wsloopOp.getPrivateNeedsBarrier())))
@@ -4820,7 +4829,8 @@ convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase &builder,
           convertToScheduleKind(schedule), chunk, isSimd,
           scheduleMod == omp::ScheduleModifier::monotonic,
           scheduleMod == omp::ScheduleModifier::nonmonotonic, isOrdered,
-          workshareLoopType, noLoopMode, hasDistSchedule, distScheduleChunk);
+          workshareLoopType, noLoopMode, hasDistSchedule, distScheduleChunk,
+          !wsloopOp.getLinearVars().empty());
 
   if (failed(handleError(wsloopIP, opInst)))
     return failure();
@@ -4938,7 +4948,7 @@ convertOmpParallel(omp::ParallelOp opInst, llvm::IRBuilderBase &builder,
             .failed())
       return llvm::make_error<PreviouslyReportedError>();
 
-    if (failed(copyFirstPrivateVars(
+    if (failed(completePrivateVars(
             opInst, builder, moduleTranslation, privateVarsInfo.mlirVars,
             privateVarsInfo.llvmVars, privateVarsInfo.privatizers,
             opInst.getPrivateNeedsBarrier())))
@@ -5177,7 +5187,7 @@ convertOmpSimd(Operation &opInst, llvm::IRBuilderBase &builder,
           .failed())
     return failure();
 
-  // No call to copyFirstPrivateVars because FIRSTPRIVATE is not allowed for
+  // No call to completePrivateVars because FIRSTPRIVATE is not allowed for
   // SIMD.
 
   assert(afterAllocas.get()->getSinglePredecessor());
@@ -8717,10 +8727,10 @@ convertOmpDistribute(Operation &opInst, llvm::IRBuilderBase &builder,
             .failed())
       return llvm::make_error<PreviouslyReportedError>();
 
-    if (failed(copyFirstPrivateVars(
-            distributeOp, builder, moduleTranslation, privVarsInfo.mlirVars,
-            privVarsInfo.llvmVars, privVarsInfo.privatizers,
-            distributeOp.getPrivateNeedsBarrier())))
+    if (failed(completePrivateVars(distributeOp, builder, moduleTranslation,
+                                   privVarsInfo.mlirVars, privVarsInfo.llvmVars,
+                                   privVarsInfo.privatizers,
+                                   distributeOp.getPrivateNeedsBarrier())))
       return llvm::make_error<PreviouslyReportedError>();
 
     llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
@@ -9601,7 +9611,7 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
             .failed())
       return llvm::make_error<PreviouslyReportedError>();
 
-    if (failed(copyFirstPrivateVars(
+    if (failed(completePrivateVars(
             targetOp, builder, moduleTranslation, privateVarsInfo.mlirVars,
             privateVarsInfo.llvmVars, privateVarsInfo.privatizers,
             targetOp.getPrivateNeedsBarrier(), &mappedPrivateVars)))
diff --git a/mlir/test/Target/LLVMIR/omptarget-wsloop.mlir b/mlir/test/Target/LLVMIR/omptarget-wsloop.mlir
index acbb3ec916113..a14efc9ae84f8 100644
--- a/mlir/test/Target/LLVMIR/omptarget-wsloop.mlir
+++ b/mlir/test/Target/LLVMIR/omptarget-wsloop.mlir
@@ -29,6 +29,32 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"dlti.alloca_memo
       }
     llvm.return
   }
+
+  llvm.func @target_wsloop_nowait(%arg0: !llvm.ptr) attributes {omp.declare_target = #omp.declaretarget<device_type = any, capture_clause = to>} {
+      %loop_ub = llvm.mlir.constant(9 : i32) : i32
+      %loop_lb = llvm.mlir.constant(0 : i32) : i32
+      %loop_step = llvm.mlir.constant(1 : i32) : i32
+      omp.wsloop nowait {
+        omp.loop_nest (%loop_cnt) : i32 = (%loop_lb) to (%loop_ub) inclusive step (%loop_step) {
+          %gep = llvm.getelementptr %arg0[0, %loop_cnt] : (!llvm.ptr, i32) -> !llvm.ptr, !llvm.array<10 x i32>
+          llvm.store %loop_cnt, %gep : i32, !llvm.ptr
+          omp.yield
+        }
+      }
+    llvm.return
+  }
+
+  llvm.fun...
[truncated]

llvmorg-github-actions · 2026-09-16T15:00:04Z

@llvm/pr-subscribers-mlir

Author: Nicole Aschenbrenner (nicebert)

Changes

Emitting loop-free kernels for no-loop target regions in Clang requires
the shared OpenMP lowering to honor flags that reach it today and are
then discarded.

applyWorkshareLoop takes a NeedsBarrier flag, but the device path drops
it, so a worksharing loop without nowait emits no barrier at its exit.
The device path also never sets the canonical loop's last iteration
variable, which the linear clause finalization reads. Forward the flag
to applyWorkshareLoopTarget and compute the last iteration in the loop
body, mirroring how the host runtime reports it.

Auditing the surrounding lowering for the same class of problem turned
up one more. The barrier that follows privatization is emitted as part
of the firstprivate copy region, so a construct with lastprivate and no
firstprivate never gets one. Emit it independently of the copy region.
Flang skips it for taskloop, where the write-back already happens after
the reads.

Split out of #205325.

Patch is 22.20 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/224040.diff

7 Files Affected:

(modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+7)
(added) flang/test/Integration/OpenMP/privatization-barrier.f90 (+38)
(modified) flang/test/Lower/OpenMP/taskloop.f90 (+15)
(modified) llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h (+10-6)
(modified) llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp (+38-4)
(modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+53-43)
(modified) mlir/test/Target/LLVMIR/omptarget-wsloop.mlir (+40)

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 8d0d191058cfb..3e63c068e1cdb 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -372,6 +372,13 @@ bool DataSharingProcessor::needBarrier() {
   // initialization of firstprivate variables and post-update of lastprivate
   // variables.
   // Emit implicit barrier for linear clause in the OpenMPIRBuilder.
+  // Skip for taskloop: the write-back only happens after the reads are done.
+
+  const auto *ompEval = eval.getIf<parser::OpenMPConstruct>();
+  if (ompEval && llvm::omp::allTaskloopSet.test(
+                     parser::omp::GetOmpDirectiveName(*ompEval).v))
+    return false;
+
   for (const semantics::Symbol *sym : allPrivatizedSymbols) {
     if (sym->test(semantics::Symbol::Flag::OmpLastPrivate) &&
         (sym->test(semantics::Symbol::Flag::OmpFirstPrivate) ||
diff --git a/flang/test/Integration/OpenMP/privatization-barrier.f90 b/flang/test/Integration/OpenMP/privatization-barrier.f90
new file mode 100644
index 0000000000000..06ab26df2521e
--- /dev/null
+++ b/flang/test/Integration/OpenMP/privatization-barrier.f90
@@ -0,0 +1,38 @@
+!===----------------------------------------------------------------------===!
+! This directory can be used to add Integration tests involving multiple
+! stages of the compiler (for eg. from Fortran to LLVM IR). It should not
+! contain executable tests. We should only add tests here sparingly and only
+! if there is no other way to test. Repeat this message in each test that is
+! added to this directory and sub-directories.
+!===----------------------------------------------------------------------===!
+
+! RUN: %flang_fc1 -fopenmp -emit-llvm %s -o - | FileCheck %s
+! RUN: %if amdgpu-registered-target %{ %flang_fc1 -triple amdgcn-amd-amdhsa -emit-llvm -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s %}
+
+subroutine lastprivate_allocatable_barrier_host
+  integer, allocatable :: a
+  integer :: i
+  !$omp parallel do lastprivate(a)
+  do i = 1, 10
+    a = i
+  end do
+  !$omp end parallel do
+end subroutine
+
+subroutine lastprivate_allocatable_barrier_device
+  integer, allocatable :: a
+  integer :: i
+  allocate(a)
+  !$omp target parallel do lastprivate(a)
+  do i = 1, 10
+    a = i
+  end do
+  !$omp end target parallel do
+end subroutine
+
+! CHECK-LABEL: define internal void @{{.*}}lastprivate_allocatable_barrier_{{(host|device)}}
+! CHECK:         call void @__kmpc_barrier
+! CHECK-NEXT:    br label %omp.wsloop.region
+! CHECK:         call void @__kmpc_barrier
+! CHECK-NEXT:    br label %omp_loop.after
+! CHECK-LABEL: define{{.*}}void @{{.*}}lastprivate_allocatable_barrier_device
diff --git a/flang/test/Lower/OpenMP/taskloop.f90 b/flang/test/Lower/OpenMP/taskloop.f90
index 94e4e2947fa71..c9ce984db7e8e 100644
--- a/flang/test/Lower/OpenMP/taskloop.f90
+++ b/flang/test/Lower/OpenMP/taskloop.f90
@@ -2,6 +2,9 @@
 ! RUN: bbc -emit-hlfir %openmp_flags -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s
 ! RUN: %flang_fc1 -emit-hlfir %openmp_flags -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s
 
+! CHECK-LABEL:  omp.private {type = firstprivate}
+! CHECK-SAME:     @[[FIRST_LAST_PRIVATE_X:.*]] : i32
+
 ! CHECK-LABEL:  omp.private
 ! CHECK-SAME:       {type = private} @[[LAST_PRIVATE_I:.*]] : i32
 
@@ -260,3 +263,15 @@ subroutine omp_taskloop_lastprivate()
    ! CHECK:  omp.terminator
    !$omp end taskloop
 end subroutine omp_taskloop_lastprivate
+
+! CHECK-LABEL:  func @_QPomp_taskloop_first_and_lastprivate()
+subroutine omp_taskloop_first_and_lastprivate()
+   integer x
+   x = 0
+   ! CHECK:  omp.taskloop.context private(@[[FIRST_LAST_PRIVATE_X]] {{.*}}) {
+   !$omp taskloop firstprivate(x) lastprivate(x)
+   do i = 1, 100
+      x = x + 1
+   end do
+   !$omp end taskloop
+end subroutine omp_taskloop_first_and_lastprivate
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
index a13832d69b6a0..90a91e98b4863 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -1177,15 +1177,19 @@ class OpenMPIRBuilder {
   /// \param CLI      A descriptor of the canonical loop to workshare.
   /// \param AllocaIP An insertion point for Alloca instructions usable in the
   ///                 preheader of the loop.
+  /// \param NeedsBarrier Indicates whether a barrier must be inserted after
+  ///                     the loop.
   /// \param LoopType Information about type of loop worksharing.
   ///                 It corresponds to type of loop workshare OpenMP pragma.
   /// \param NoLoop   If true, no-loop code is generated.
+  /// \param NeedsLastIter  If true, the last iteration variable is emitted.
   ///
   /// \returns Point where to insert code after the workshare construct.
-  InsertPointTy applyWorkshareLoopTarget(DebugLoc DL, CanonicalLoopInfo *CLI,
-                                         InsertPointTy AllocaIP,
-                                         omp::WorksharingLoopType LoopType,
-                                         bool NoLoop);
+  InsertPointOrErrorTy
+  applyWorkshareLoopTarget(DebugLoc DL, CanonicalLoopInfo *CLI,
+                           InsertPointTy AllocaIP, bool NeedsBarrier,
+                           omp::WorksharingLoopType LoopType, bool NoLoop,
+                           bool NeedsLastIter);
 
   /// Modifies the canonical loop to be a statically-scheduled workshare loop.
   ///
@@ -1343,8 +1347,8 @@ class OpenMPIRBuilder {
   /// \param NoLoop If true, no-loop code is generated.
   /// \param HasDistSchedule Defines if the clause being lowered is
   /// dist_schedule as this is handled slightly differently
-  ///
   /// \param DistScheduleChunkSize The chunk size for dist_schedule loop
+  /// \param NeedsLastIter  If true, the last iteration variable is emitted.
   ///
   /// \returns Point where to insert code after the workshare construct.
   LLVM_ABI InsertPointOrErrorTy applyWorkshareLoop(
@@ -1357,7 +1361,7 @@ class OpenMPIRBuilder {
       omp::WorksharingLoopType LoopType =
           omp::WorksharingLoopType::ForStaticLoop,
       bool NoLoop = false, bool HasDistSchedule = false,
-      Value *DistScheduleChunkSize = nullptr);
+      Value *DistScheduleChunkSize = nullptr, bool NeedsLastIter = false);
 
   /// Tile a loop nest.
   ///
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 0747b943d1ba4..8741ccba37d08 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -6612,11 +6612,32 @@ static void workshareLoopTargetCallback(
   CLI->invalidate();
 }
 
-OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::applyWorkshareLoopTarget(
+OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::applyWorkshareLoopTarget(
     DebugLoc DL, CanonicalLoopInfo *CLI, InsertPointTy AllocaIP,
-    WorksharingLoopType LoopType, bool NoLoop) {
+    bool NeedsBarrier, WorksharingLoopType LoopType, bool NoLoop,
+    bool NeedsLastIter) {
   uint32_t SrcLocStrSize;
   Constant *SrcLocStr = getOrCreateSrcLocStr(DL, SrcLocStrSize);
+
+  // Mirrors host runtime reporting of last iteration by in-body computation.
+  if (NeedsLastIter) {
+    Type *I32Type = Type::getInt32Ty(M.getContext());
+    Builder.restoreIP(AllocaIP);
+    AllocaInst *PLastIter =
+        Builder.CreateAlloca(I32Type, nullptr, "p.lastiter");
+    Builder.CreateStore(ConstantInt::get(I32Type, 0), PLastIter);
+    CLI->setLastIter(PLastIter);
+
+    Builder.SetInsertPoint(CLI->getBody(),
+                           CLI->getBody()->getFirstInsertionPt());
+    Value *TripCount = CLI->getTripCount();
+    Value *LastIter =
+        Builder.CreateSub(TripCount, ConstantInt::get(TripCount->getType(), 1));
+    Value *IsLast =
+        Builder.CreateICmpEQ(CLI->getIndVar(), LastIter, "omp.is_last_iter");
+    Builder.CreateStore(Builder.CreateZExt(IsLast, I32Type), PLastIter);
+  }
+
   IdentFlag Flag = IdentFlag(0);
   switch (LoopType) {
   case WorksharingLoopType::ForStaticLoop:
@@ -6713,6 +6734,18 @@ OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::applyWorkshareLoopTarget(
                                 LoopType, NoLoop);
   };
   addOutlineInfo(std::move(OI));
+
+  if (NeedsBarrier) {
+    Builder.SetInsertPoint(CLI->getExit(),
+                           CLI->getExit()->getTerminator()->getIterator());
+    InsertPointOrErrorTy BarrierIP =
+        createBarrier(LocationDescription(Builder.saveIP(), DL),
+                      omp::Directive::OMPD_for, /* ForceSimpleCall */ false,
+                      /* CheckCancelFlag*/ false);
+    if (!BarrierIP)
+      return BarrierIP.takeError();
+  }
+
   return CLI->getAfterIP();
 }
 
@@ -6722,9 +6755,10 @@ OpenMPIRBuilder::InsertPointOrErrorTy OpenMPIRBuilder::applyWorkshareLoop(
     bool HasSimdModifier, bool HasMonotonicModifier,
     bool HasNonmonotonicModifier, bool HasOrderedClause,
     WorksharingLoopType LoopType, bool NoLoop, bool HasDistSchedule,
-    Value *DistScheduleChunkSize) {
+    Value *DistScheduleChunkSize, bool NeedsLastIter) {
   if (Config.isTargetDevice())
-    return applyWorkshareLoopTarget(DL, CLI, AllocaIP, LoopType, NoLoop);
+    return applyWorkshareLoopTarget(DL, CLI, AllocaIP, NeedsBarrier, LoopType,
+                                    NoLoop, NeedsLastIter);
   OMPScheduleType EffectiveScheduleType = computeOpenMPScheduleType(
       SchedKind, ChunkSize, HasSimdModifier, HasMonotonicModifier,
       HasNonmonotonicModifier, HasOrderedClause, DistScheduleChunkSize);
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index e5e3e0cc6d944..f6a4b4c5d7f3f 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -2083,13 +2083,27 @@ static bool opIsInSingleThread(mlir::Operation *op) {
   return false;
 }
 
-static LogicalResult copyFirstPrivateVars(
-    mlir::Operation *op, llvm::IRBuilderBase &builder,
-    LLVM::ModuleTranslation &moduleTranslation,
-    SmallVectorImpl<llvm::Value *> &moldVars,
-    ArrayRef<llvm::Value *> llvmPrivateVars,
-    SmallVectorImpl<omp::PrivateClauseOp> &privateDecls, bool insertBarrier,
-    llvm::DenseMap<Value, Value> *mappedPrivateVars = nullptr) {
+static LogicalResult
+emitPrivatizationBarrier(mlir::Operation *op, llvm::IRBuilderBase &builder,
+                         LLVM::ModuleTranslation &moduleTranslation,
+                         bool insertBarrier) {
+  if (!insertBarrier || opIsInSingleThread(op))
+    return success();
+
+  llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
+  llvm::OpenMPIRBuilder::InsertPointOrErrorTy res =
+      ompBuilder->createBarrier(builder, llvm::omp::OMPD_barrier);
+  return handleError(res, *op);
+}
+
+static LogicalResult
+completePrivateVars(mlir::Operation *op, llvm::IRBuilderBase &builder,
+                    LLVM::ModuleTranslation &moduleTranslation,
+                    SmallVectorImpl<llvm::Value *> &moldVars,
+                    ArrayRef<llvm::Value *> llvmPrivateVars,
+                    SmallVectorImpl<omp::PrivateClauseOp> &privateDecls,
+                    bool insertBarrier,
+                    llvm::DenseMap<Value, Value> *mappedPrivateVars = nullptr) {
   // Apply copy region for firstprivate.
   bool needsFirstprivate =
       llvm::any_of(privateDecls, [](omp::PrivateClauseOp &privOp) {
@@ -2098,7 +2112,8 @@ static LogicalResult copyFirstPrivateVars(
       });
 
   if (!needsFirstprivate)
-    return success();
+    return emitPrivatizationBarrier(op, builder, moduleTranslation,
+                                    insertBarrier);
 
   llvm::BasicBlock *copyBlock =
       splitBB(builder, /*CreateBranch=*/true, "omp.private.copy");
@@ -2137,24 +2152,18 @@ static LogicalResult copyFirstPrivateVars(
     moduleTranslation.forgetMapping(copyRegion);
   }
 
-  if (insertBarrier && !opIsInSingleThread(op)) {
-    llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
-    llvm::OpenMPIRBuilder::InsertPointOrErrorTy res =
-        ompBuilder->createBarrier(builder, llvm::omp::OMPD_barrier);
-    if (failed(handleError(res, *op)))
-      return failure();
-  }
-
-  return success();
+  return emitPrivatizationBarrier(op, builder, moduleTranslation,
+                                  insertBarrier);
 }
 
-static LogicalResult copyFirstPrivateVars(
-    mlir::Operation *op, llvm::IRBuilderBase &builder,
-    LLVM::ModuleTranslation &moduleTranslation,
-    SmallVectorImpl<mlir::Value> &mlirPrivateVars,
-    ArrayRef<llvm::Value *> llvmPrivateVars,
-    SmallVectorImpl<omp::PrivateClauseOp> &privateDecls, bool insertBarrier,
-    llvm::DenseMap<Value, Value> *mappedPrivateVars = nullptr) {
+static LogicalResult
+completePrivateVars(mlir::Operation *op, llvm::IRBuilderBase &builder,
+                    LLVM::ModuleTranslation &moduleTranslation,
+                    SmallVectorImpl<mlir::Value> &mlirPrivateVars,
+                    ArrayRef<llvm::Value *> llvmPrivateVars,
+                    SmallVectorImpl<omp::PrivateClauseOp> &privateDecls,
+                    bool insertBarrier,
+                    llvm::DenseMap<Value, Value> *mappedPrivateVars = nullptr) {
   llvm::SmallVector<llvm::Value *> moldVars(mlirPrivateVars.size());
   llvm::transform(mlirPrivateVars, moldVars.begin(), [&](mlir::Value mlirVar) {
     // map copyRegion rhs arg
@@ -2163,9 +2172,9 @@ static LogicalResult copyFirstPrivateVars(
     assert(moldVar);
     return moldVar;
   });
-  return copyFirstPrivateVars(op, builder, moduleTranslation, moldVars,
-                              llvmPrivateVars, privateDecls, insertBarrier,
-                              mappedPrivateVars);
+  return completePrivateVars(op, builder, moduleTranslation, moldVars,
+                             llvmPrivateVars, privateDecls, insertBarrier,
+                             mappedPrivateVars);
 }
 
 template <typename T>
@@ -2419,7 +2428,7 @@ convertOmpScope(omp::ScopeOp &scopeOp, llvm::IRBuilderBase &builder,
             .failed())
       return llvm::make_error<PreviouslyReportedError>();
 
-    if (failed(copyFirstPrivateVars(
+    if (failed(completePrivateVars(
             scopeOp, builder, moduleTranslation, privateVarsInfo.mlirVars,
             privateVarsInfo.llvmVars, privateVarsInfo.privatizers,
             scopeOp.getPrivateNeedsBarrier())))
@@ -3340,7 +3349,7 @@ convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
 
   // firstprivate copy region
   setInsertPointForPossiblyEmptyBlock(builder, copyBlock);
-  if (failed(copyFirstPrivateVars(
+  if (failed(completePrivateVars(
           taskOp, builder, moduleTranslation, privateVarsInfo.mlirVars,
           taskStructMgr.getLLVMPrivateVarGEPs(), privateVarsInfo.privatizers,
           taskOp.getPrivateNeedsBarrier())))
@@ -3811,7 +3820,7 @@ convertOmpTaskloopContextOp(omp::TaskloopContextOp contextOp,
 
   // firstprivate copy region
   setInsertPointForPossiblyEmptyBlock(builder, copyBlock);
-  if (failed(copyFirstPrivateVars(
+  if (failed(completePrivateVars(
           contextOp, builder, moduleTranslation, privateVarsInfo.mlirVars,
           taskStructMgr.getLLVMPrivateVarGEPs(), privateVarsInfo.privatizers,
           contextOp.getPrivateNeedsBarrier())))
@@ -4110,10 +4119,10 @@ convertOmpTaskloopContextOp(omp::TaskloopContextOp contextOp,
       // through a stack allocated structure.
     }
 
-    if (failed(copyFirstPrivateVars(contextOp.getOperation(), builder,
-                                    moduleTranslation, srcGEPs, destGEPs,
-                                    privateVarsInfo.privatizers,
-                                    contextOp.getPrivateNeedsBarrier())))
+    if (failed(completePrivateVars(contextOp.getOperation(), builder,
+                                   moduleTranslation, srcGEPs, destGEPs,
+                                   privateVarsInfo.privatizers,
+                                   contextOp.getPrivateNeedsBarrier())))
       return llvm::make_error<PreviouslyReportedError>();
 
     return builder.saveIP();
@@ -4707,7 +4716,7 @@ convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase &builder,
           .failed())
     return failure();
 
-  if (failed(copyFirstPrivateVars(
+  if (failed(completePrivateVars(
           wsloopOp, builder, moduleTranslation, privateVarsInfo.mlirVars,
           privateVarsInfo.llvmVars, privateVarsInfo.privatizers,
           wsloopOp.getPrivateNeedsBarrier())))
@@ -4820,7 +4829,8 @@ convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase &builder,
           convertToScheduleKind(schedule), chunk, isSimd,
           scheduleMod == omp::ScheduleModifier::monotonic,
           scheduleMod == omp::ScheduleModifier::nonmonotonic, isOrdered,
-          workshareLoopType, noLoopMode, hasDistSchedule, distScheduleChunk);
+          workshareLoopType, noLoopMode, hasDistSchedule, distScheduleChunk,
+          !wsloopOp.getLinearVars().empty());
 
   if (failed(handleError(wsloopIP, opInst)))
     return failure();
@@ -4938,7 +4948,7 @@ convertOmpParallel(omp::ParallelOp opInst, llvm::IRBuilderBase &builder,
             .failed())
       return llvm::make_error<PreviouslyReportedError>();
 
-    if (failed(copyFirstPrivateVars(
+    if (failed(completePrivateVars(
             opInst, builder, moduleTranslation, privateVarsInfo.mlirVars,
             privateVarsInfo.llvmVars, privateVarsInfo.privatizers,
             opInst.getPrivateNeedsBarrier())))
@@ -5177,7 +5187,7 @@ convertOmpSimd(Operation &opInst, llvm::IRBuilderBase &builder,
           .failed())
     return failure();
 
-  // No call to copyFirstPrivateVars because FIRSTPRIVATE is not allowed for
+  // No call to completePrivateVars because FIRSTPRIVATE is not allowed for
   // SIMD.
 
   assert(afterAllocas.get()->getSinglePredecessor());
@@ -8717,10 +8727,10 @@ convertOmpDistribute(Operation &opInst, llvm::IRBuilderBase &builder,
             .failed())
       return llvm::make_error<PreviouslyReportedError>();
 
-    if (failed(copyFirstPrivateVars(
-            distributeOp, builder, moduleTranslation, privVarsInfo.mlirVars,
-            privVarsInfo.llvmVars, privVarsInfo.privatizers,
-            distributeOp.getPrivateNeedsBarrier())))
+    if (failed(completePrivateVars(distributeOp, builder, moduleTranslation,
+                                   privVarsInfo.mlirVars, privVarsInfo.llvmVars,
+                                   privVarsInfo.privatizers,
+                                   distributeOp.getPrivateNeedsBarrier())))
       return llvm::make_error<PreviouslyReportedError>();
 
     llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
@@ -9601,7 +9611,7 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
             .failed())
       return llvm::make_error<PreviouslyReportedError>();
 
-    if (failed(copyFirstPrivateVars(
+    if (failed(completePrivateVars(
             targetOp, builder, moduleTranslation, privateVarsInfo.mlirVars,
             privateVarsInfo.llvmVars, privateVarsInfo.privatizers,
             targetOp.getPrivateNeedsBarrier(), &mappedPrivateVars)))
diff --git a/mlir/test/Target/LLVMIR/omptarget-wsloop.mlir b/mlir/test/Target/LLVMIR/omptarget-wsloop.mlir
index acbb3ec916113..a14efc9ae84f8 100644
--- a/mlir/test/Target/LLVMIR/omptarget-wsloop.mlir
+++ b/mlir/test/Target/LLVMIR/omptarget-wsloop.mlir
@@ -29,6 +29,32 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"dlti.alloca_memo
       }
     llvm.return
   }
+
+  llvm.func @target_wsloop_nowait(%arg0: !llvm.ptr) attributes {omp.declare_target = #omp.declaretarget<device_type = any, capture_clause = to>} {
+      %loop_ub = llvm.mlir.constant(9 : i32) : i32
+      %loop_lb = llvm.mlir.constant(0 : i32) : i32
+      %loop_step = llvm.mlir.constant(1 : i32) : i32
+      omp.wsloop nowait {
+        omp.loop_nest (%loop_cnt) : i32 = (%loop_lb) to (%loop_ub) inclusive step (%loop_step) {
+          %gep = llvm.getelementptr %arg0[0, %loop_cnt] : (!llvm.ptr, i32) -> !llvm.ptr, !llvm.array<10 x i32>
+          llvm.store %loop_cnt, %gep : i32, !llvm.ptr
+          omp.yield
+        }
+      }
+    llvm.return
+  }
+
+  llvm.fun...
[truncated]

tblah

Thanks for the patch

tblah · 2026-09-21T09:16:19Z

+  // No call to completePrivateVars because FIRSTPRIVATE is not allowed for
  // SIMD.


Is this still correct now that completePrivateVars has a dual purpose?

Good point. I've updated the comment to account for the dual purpose, but the premise still holds: we don't need to call it for simd.

tblah · 2026-09-21T09:20:09Z

@@ -0,0 +1,38 @@
+!===----------------------------------------------------------------------===!
+! This directory can be used to add Integration tests involving multiple


I'm not sure this needs to be an integration test (feel free to explain why I am mistaken).

Please could you just check the change in the MLIR produced by flang due to the change in DataSharingProcessor? flang -fc1 -emit-hlfir -fopenmp -o - ... similar to existing tests in flang/test/Lower/OpenMP

The HLFIR test you propose would not test the fix: Flang already emits private_barrier for this case before the patch, and that is already checked by lastprivate-allocatable.f90. The bug was in the translation to LLVM IR, which ignored the barrier when there was no firstprivate. Instead of the integration test I could've widened openmp-private-barrier-single.mlir to cover the lastprivate-only case, but I preferred a test that goes through both stages, so the translation is tested on what Flang actually emits for this code, not on hand-written MLIR.

tblah · 2026-09-21T10:33:37Z

+    Builder.restoreIP(AllocaIP);
+    AllocaInst *PLastIter =
+        Builder.CreateAlloca(I32Type, nullptr, "p.lastiter");
+    Builder.CreateStore(ConstantInt::get(I32Type, 0), PLastIter);


The workshare loop might be executed multiple times but PLastIter is only initialised once. This store should probably be immediately before the loop.

You're right, I've fixed it. The store now sits at the end of the preheader, so it's reset every time.

skatrak

Thanks Nicole, only some small comments from me.

skatrak · 2026-09-23T12:39:03Z

+subroutine omp_taskloop_first_and_lastprivate()
+   integer x
+   x = 0
+   ! CHECK:  omp.taskloop.context private(@[[FIRST_LAST_PRIVATE_X]] {{.*}}) {


I think this test should also check that a barrier isn't added either as an omp.barrier op or through a private_barrier attribute, since that's the behavior being changed.

The existing check already covers this: the private_barrier attribute would be emitted between the closing ) and the opening { of the region, so requiring ) { fails if it's there. I can make that explicit if you prefer, which would be:

! CHECK: omp.taskloop.context private(@[[FIRST_LAST_PRIVATE_X]] {{.*}}) ! CHECK-NOT: private_barrier ! CHECK-SAME: {

skatrak · 2026-09-24T11:49:06Z

+emitPrivatizationBarrier(mlir::Operation *op, llvm::IRBuilderBase &builder,
+                         LLVM::ModuleTranslation &moduleTranslation,
+                         bool insertBarrier) {
+  if (!insertBarrier || opIsInSingleThread(op))


Nit: I don't think it's a good idea to make insertBarrier an argument to this function, whose only purpose is to create a barrier. Whenever we know we don't need to add one, just don't call it.

Fair. emitPrivatizationBarrier no longer takes insertBarrier, and the callers only call it when a barrier is needed.

llvmorg-github-actions Bot added mlir:llvm mlir flang Flang issues not falling into any other category mlir:openmp flang:fir-hlfir flang:openmp clang:openmp OpenMP related changes to Clang labels Sep 16, 2026

This was referenced Sep 16, 2026

[clang][OpenMP] Emit lastprivate final copies in no-loop kernels #224045

Open

[OpenMP] Add no-loop SPMD kernel emission to Clang #205325

Closed

tblah reviewed Sep 21, 2026

View reviewed changes

nicebert requested a review from skatrak September 22, 2026 13:05

skatrak reviewed Sep 24, 2026

View reviewed changes

Reset last iteration on every loop entry, simplify barrier helper

9b07a8f

nicebert requested review from skatrak and tblah September 25, 2026 08:44

		// No call to completePrivateVars because FIRSTPRIVATE is not allowed for
		// SIMD.

		@@ -0,0 +1,38 @@
		!===----------------------------------------------------------------------===!
		! This directory can be used to add Integration tests involving multiple

Conversation

nicebert commented Sep 16, 2026

Uh oh!

llvmorg-github-actions Bot commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmorg-github-actions Bot commented Sep 16, 2026

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skatrak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

llvmorg-github-actions Bot commented Sep 16, 2026 •

edited

Loading