[flang][OpenMP] Fix wrong results for FORALL in a workshare construct (#211371) A FORALL in a workshare construct could produce wrong results non-deterministically. This is caused by two issues in the workshare lowering: 1. A FORALL whose left-hand side may overlap its right-hand side is lowered into two loop nests around a runtime value stack: the first nest evaluates each right-hand side and pushes it, the second one fetches the saved values back with a running counter. That counter lives in a fir.alloca which, since omp.parallel is an alloca scope, is thread private. The counter is read, incremented and written back from inside the omp.single generated for the fetch, because the incremented value is only available there. Only the thread which executed the omp.single therefore bumped its own copy of the counter, and all the other threads kept a stale one and refetched an already consumed element on the following iterations. Collect the thread local memory which is only updated by the thread executing an omp.single and broadcast it with copyprivate, so that the copies of the other threads stay in sync. As nowait and copyprivate are mutually exclusive on a single construct, nowait is no longer set when there is something to broadcast. 2. nowait was only suppressed when the immediately enclosing operation was loop-like. A masked FORALL introduces a fir.if inside the fir.do_loop, so the last omp.single or omp.wsloop of the fir.if body was given nowait even though the loop may run it again, and even though there was more work after the loop. Thread the information down the recursion instead, so that only the work which is really last in the whole omp.workshare region may rely on the barrier emitted at the end of that region. Fixes #209942 Fixes #209943 GitOrigin-RevId: ec62b5ae07df222f06526d5260cf6baea28c7a30
diff --git a/lib/Optimizer/OpenMP/LowerWorkshare.cpp b/lib/Optimizer/OpenMP/LowerWorkshare.cpp index 2bc8a4b..aff250c 100644 --- a/lib/Optimizer/OpenMP/LowerWorkshare.cpp +++ b/lib/Optimizer/OpenMP/LowerWorkshare.cpp
@@ -35,6 +35,7 @@ #include <mlir/IR/PatternMatch.h> #include <mlir/IR/Value.h> #include <mlir/IR/Visitors.h> +#include <mlir/Interfaces/CallInterfaces.h> #include <mlir/Interfaces/LoopLikeInterface.h> #include <mlir/Interfaces/SideEffectInterfaces.h> #include <mlir/Support/LLVM.h> @@ -134,6 +135,16 @@ .wasInterrupted(); } +// If val is defined by an hlfir.declare/fir.declare, returns the declared +// memref (the value the declare wraps); otherwise returns val unchanged. +static Value lookThroughDeclare(Value val) { + if (auto hlfirDecl = val.getDefiningOp<hlfir::DeclareOp>()) + return hlfirDecl.getMemref(); + if (auto firDecl = val.getDefiningOp<fir::DeclareOp>()) + return firDecl.getMemref(); + return val; +} + // Determines if a memory reference is thread-local in an OpenMP context. // // This is a best-effort analysis. We cannot definitively determine if code @@ -174,19 +185,13 @@ // sets the source value to the declare op result (not the block arg). // Trace through the declare to check if the underlying Memref is a // private block argument. - Value declMemref; - if (auto hlfirDecl = sourceValue.getDefiningOp<hlfir::DeclareOp>()) - declMemref = hlfirDecl.getMemref(); - else if (auto firDecl = sourceValue.getDefiningOp<fir::DeclareOp>()) - declMemref = firDecl.getMemref(); - if (declMemref) { - if (auto blockArg = llvm::dyn_cast<BlockArgument>(declMemref)) { - Operation *parentOp = blockArg.getOwner()->getParentOp(); - if (auto argIface = - llvm::dyn_cast<omp::BlockArgOpenMPOpInterface>(parentOp)) { - if (llvm::is_contained(argIface.getPrivateBlockArgs(), blockArg)) - return true; - } + if (auto blockArg = + llvm::dyn_cast<BlockArgument>(lookThroughDeclare(sourceValue))) { + Operation *parentOp = blockArg.getOwner()->getParentOp(); + if (auto argIface = + llvm::dyn_cast<omp::BlockArgOpenMPOpInterface>(parentOp)) { + if (llvm::is_contained(argIface.getPrivateBlockArgs(), blockArg)) + return true; } } } @@ -259,6 +264,145 @@ return false; } +// Returns the underlying thread-local storage that mem refers to, or null if +// mem is not thread-local. The alias analysis is used to look through +// fir.declare/hlfir.declare, fir.convert, fir.rebox, etc., so that two +// accesses of the same thread-local location yield the same value even if one +// goes through such ops and the other does not. This is what makes it safe to +// match the reads and writes of collect{Reads,Writes} against each other by +// value identity: a store to an alloca and a load from a fir.declare of that +// alloca map to the same key. Matching the raw effect value instead would +// silently miss such accesses, dropping a required broadcast. +static Value getOpenMPThreadLocalSource(Operation *op, Value mem) { + if (!isOpenMPThreadLocalMemory(op, mem)) + return nullptr; + fir::AliasAnalysis aliasAnalysis; + Value source = llvm::dyn_cast_if_present<mlir::Value>( + aliasAnalysis.getSource(mem).origin.u); + if (!source) + return nullptr; + // The alias analysis looks through a fir.declare/hlfir.declare that wraps an + // allocation, but stops at the declare result when it wraps a privatizing + // clause block argument (see isOpenMPThreadLocalMemory). A write through such + // a declare and a read of the block argument itself (or vice versa) would + // then result in different origins and fail to match, dropping a required + // broadcast. Look through the declare to the block argument so that both + // accesses canonicalize to the same key. + if (Value memref = lookThroughDeclare(source); + llvm::isa<BlockArgument>(memref)) + return memref; + return source; +} + +// Collects the thread-local memory locations that op writes to and that +// need to be broadcasted to other threads when op ends up being executed +// by a single thread only. +// +// Some thread-local variables carry state which is logically shared by the +// whole omp.workshare region even though each thread owns a copy of it. +// +// One example is the fetch counter of the temporary storage used to implement +// FORALL: it is bumped from within an omp.single (because the value it is +// bumped by is only available there), so the copies owned by the threads +// which did not execute the omp.single would otherwise go stale and the +// following iterations would fetch the wrong element. See issue #209942. +// +// Only the underlying thread-local allocation is considered, so that a shallow +// copy of it faithfully reproduces the update on the other threads. +static void collectThreadLocalWrites(Operation *op, + llvm::SmallVectorImpl<Value> &vars) { + auto memEffects = dyn_cast<MemoryEffectOpInterface>(op); + if (!memEffects) + return; + SmallVector<MemoryEffects::EffectInstance> effects; + memEffects.getEffects(effects); + for (const MemoryEffects::EffectInstance &effect : effects) { + if (!isa<MemoryEffects::Write>(effect.getEffect())) + continue; + Value val = effect.getValue(); + if (!val) + continue; + Value source = getOpenMPThreadLocalSource(op, val); + if (!source) + continue; + auto refTy = dyn_cast<fir::ReferenceType>(source.getType()); + if (!refTy) + continue; + // createCopyFunc emits a load/store pair, so restrict this to types for + // which such a shallow copy is both legal and cheap. + mlir::Type eleTy = refTy.getEleTy(); + if (!fir::isa_trivial(eleTy) && !fir::isa_box_type(eleTy)) + continue; + vars.push_back(source); + } +} + +// The thread-local locations that the whole team may read, used to decide +// which writes performed inside an omp.single must be broadcasted with +// copyprivate. "unknown" is set when an operation whose memory effects cannot +// be determined is found (see collectThreadLocalReads): such an operation +// might read any thread-local location, so every thread-local write then has +// to be broadcasted to stay correct. +struct ThreadLocalReads { + llvm::SmallDenseSet<Value> reads; + bool unknown = false; + + bool mayBeReadByTeam(Value v) const { return unknown || reads.contains(v); } +}; + +// Collects into reads the thread-local allocations that are read anywhere in +// scope. A thread-local location written from within an omp.single only needs +// to be broadcasted if some other thread may later read it. The scope must be +// a region executed by the whole team (i.e. the enclosing omp.parallel), so +// that reads performed after the omp.workshare region are accounted for too. +// +// Reads are matched by their underlying thread-local allocation, mirroring +// collectThreadLocalWrites, so that a load through a fir.declare/fir.convert +// still keeps the corresponding write live for broadcasting. +// +// An opaque call may read any thread-local location inside its callee, and a +// memory-effecting operation may report a read of unspecified memory (a read +// effect with no attached value). Neither read can be attributed to a specific +// location, so reads.unknown is set to force every thread-local write to be +// broadcasted. Other interface-less operations (e.g. omp.barrier, fir.declare) +// have known, inspectable behaviour and are safe to ignore here. +static void collectThreadLocalReads(Region &scope, ThreadLocalReads &reads) { + scope.walk([&](Operation *op) { + if (isa<mlir::CallOpInterface>(op)) { + reads.unknown = true; + return; + } + // TODO: An op that does not implement MemoryEffectOpInterface has + // unknown effects and could read a thread-local location, so the + // conservative choice would be to set reads.unknown here. + // However, we deliberately don't, because the interface-less ops we see in + // practice (omp.barrier, fir.declare, etc) have known, inspectable + // behaviour and never read program memory. Forcing a broadcast for them + // would defeat the read-back optimization. This assumption only holds for + // the FIR/HLFIR/OpenMP dialects we know about; an op from another dialect + // could break it. Once such ops are properly mapped (or made to model + // their effects), fall back to reads.unknown here for any remaining + // unknown-effect ops instead of ignoring them. + auto memEffects = dyn_cast<MemoryEffectOpInterface>(op); + if (!memEffects) + return; + SmallVector<MemoryEffects::EffectInstance> effects; + memEffects.getEffects(effects); + for (const MemoryEffects::EffectInstance &effect : effects) { + if (!isa<MemoryEffects::Read>(effect.getEffect())) + continue; + Value val = effect.getValue(); + if (!val) { + // A read effect without an attached value reads unspecified memory. + reads.unknown = true; + continue; + } + if (Value source = getOpenMPThreadLocalSource(op, val)) + reads.reads.insert(source); + } + }); +} + /// Simple shallow copies suffice for our purposes in this pass, so we implement /// this simpler alternative to the full fledged `createCopyFunc` in the /// frontend @@ -339,9 +483,14 @@ op.erase(); } +// canUseNowait to check whether the work generated for sourceRegion is the +// very last thing the omp.workshare region does, and thus whether the +// synchronization of its last omp.single/omp.wsloop may be left to the +// barrier emitted at the end of the omp.workshare region. static void parallelizeRegion(Region &sourceRegion, Region &targetRegion, IRMapping &rootMapping, Location loc, - mlir::DominanceInfo &di) { + mlir::DominanceInfo &di, bool canUseNowait, + const ThreadLocalReads &threadLocalReads) { OpBuilder rootBuilder(sourceRegion.getContext()); ModuleOp m = sourceRegion.getParentOfType<ModuleOp>(); OpBuilder copyFuncBuilder(m.getBodyRegion()); @@ -365,6 +514,9 @@ OpBuilder parallelBuilder) -> std::pair<bool, SmallVector<Value>> { IRMapping singleMapping = rootMapping; SmallVector<Value> copyPrivate; + // Thread-local memory updated by the single thread only, which has to be + // broadcasted to the other threads to keep their copies in sync. + SmallVector<Value> threadLocalWrites; bool allParallelized = true; for (Operation &op : llvm::make_range(sr.begin, sr.end)) { @@ -388,6 +540,9 @@ assert(llvm::all_of(op.getResults(), [&](Value v) { return !isTransitivelyUsedOutside(v, sr); })); + // The operation only runs on the thread executing the omp.single, + // so the thread-local memory it updates has to be broadcasted. + collectThreadLocalWrites(&op, threadLocalWrites); allParallelized = false; } } else if (auto alloca = dyn_cast<fir::AllocaOp>(&op)) { @@ -399,6 +554,7 @@ allParallelized = false; } else { singleBuilder.clone(op, singleMapping); + collectThreadLocalWrites(&op, threadLocalWrites); // Prepare reloaded values for results of operations that cannot be // safely parallelized and which are used after the region `sr`. for (auto res : op.getResults()) { @@ -413,6 +569,22 @@ } } omp::TerminatorOp::create(singleBuilder, loc); + + // Broadcast the thread-local state which only the thread executing the + // omp.single has updated, but only when some other thread may actually read + // it back: a location that is never read (e.g. a write to a temporary in a + // terminal omp.single) does not need to be broadcasted. Values defined + // inside sr are remapped; values defined before it (e.g. hoisted allocas) + // are used as is. + llvm::SmallDenseSet<Value> seen(copyPrivate.begin(), copyPrivate.end()); + for (Value v : threadLocalWrites) { + if (!threadLocalReads.mayBeReadByTeam(v)) + continue; + Value mapped = singleMapping.lookupOrDefault(v); + if (seen.insert(mapped).second) + copyPrivate.push_back(mapped); + } + return {allParallelized, copyPrivate}; }; @@ -425,7 +597,7 @@ rootMapping.map(block.getArguments(), targetBlock->getArguments()); } - auto handleOneBlock = [&](Block &block) { + auto handleOneBlock = [&](Block &block, bool blockCanUseNowait) { Block &targetBlock = *rootMapping.lookup(&block); rootBuilder.setInsertionPointToStart(&targetBlock); Operation *terminator = block.getTerminator(); @@ -453,14 +625,10 @@ ; for (auto [i, opOrSingle] : llvm::enumerate(regions)) { - bool isLast = i + 1 == regions.size(); - // Make sure shared runtime calls are synchronized: disable `nowait` - // insertion, and rely on the implicit barrier at the end of the - // omp.workshare block. This applies to any loop-like operation - // (fir.do_loop, fir.iterate_while, fir.do_concurrent.loop, etc.) - // because iterations could overlap if nowait is used. - if (isa<LoopLikeOpInterface>(block.getParentOp())) - isLast = false; + // Only the very last piece of work of the whole omp.workshare region + // may use nowait and rely on the barrier emitted at the end of that + // region. + bool isLast = blockCanUseNowait && i + 1 == regions.size(); if (std::holds_alternative<SingleRegion>(opOrSingle)) { OpBuilder singleBuilder(sourceRegion.getContext()); Block *singleBlock = new Block(); @@ -485,7 +653,10 @@ delete singleBlock; } else { omp::SingleOperands singleOperands; - if (isLast) + // nowait and copyprivate are mutually exclusive on a single + // construct: the broadcast relies on the barrier at the end of the + // region. + if (isLast && copyprivateVars.empty()) singleOperands.nowait = rootBuilder.getUnitAttr(); singleOperands.copyprivateVars = copyprivateVars; cleanupBlock(singleBlock); @@ -519,10 +690,15 @@ clonedWslw->erase(); } else { assert(mustParallelizeOp(op)); + // A loop-like operation may run its region more than once, so the + // iterations of the work generated for it could overlap if nowait + // were used inside of it. + bool nestedCanUseNowait = isLast && !isa<LoopLikeOpInterface>(op); Operation *cloned = rootBuilder.cloneWithoutRegions(*op, rootMapping); for (auto [region, clonedRegion] : llvm::zip(op->getRegions(), cloned->getRegions())) - parallelizeRegion(region, clonedRegion, rootMapping, loc, di); + parallelizeRegion(region, clonedRegion, rootMapping, loc, di, + nestedCanUseNowait, threadLocalReads); } } } @@ -531,11 +707,13 @@ }; if (sourceRegion.hasOneBlock()) { - handleOneBlock(sourceRegion.front()); + handleOneBlock(sourceRegion.front(), canUseNowait); } else if (!sourceRegion.empty()) { + // With several blocks, no block is known to hold the last piece of work of + // the region, so none of them may use nowait. auto &domTree = di.getDomTree(&sourceRegion); for (auto node : llvm::breadth_first(domTree.getRootNode())) { - handleOneBlock(*node->getBlock()); + handleOneBlock(*node->getBlock(), /*blockCanUseNowait=*/false); } } @@ -595,8 +773,21 @@ if (!wsOp.getNowait()) omp::BarrierOp::create(rootBuilder, loc); - parallelizeRegion(wsOp.getRegion(), newOp.getRegion(), rootMapping, loc, - di); + // Compute the thread-local locations read by the whole team, so that only + // those get broadcasted out of the omp.single's below. The enclosing + // omp.parallel is used as the scope so that reads performed after the + // omp.workshare region are taken into account as well; if there is none, + // fall back to the innermost isolated-from-above ancestor. + ThreadLocalReads threadLocalReads; + if (auto parallelOp = wsOp->getParentOfType<omp::ParallelOp>()) + collectThreadLocalReads(parallelOp.getRegion(), threadLocalReads); + else if (Operation *top = + wsOp->getParentWithTrait<OpTrait::IsIsolatedFromAbove>()) + for (Region &r : top->getRegions()) + collectThreadLocalReads(r, threadLocalReads); + + parallelizeRegion(wsOp.getRegion(), newOp.getRegion(), rootMapping, loc, di, + /*canUseNowait=*/true, threadLocalReads); // Inline the contents of the placeholder workshare op into its parent // block.
diff --git a/test/Transforms/OpenMP/lower-workshare-nowait.mlir b/test/Transforms/OpenMP/lower-workshare-nowait.mlir index 940662e..ced4f3f 100644 --- a/test/Transforms/OpenMP/lower-workshare-nowait.mlir +++ b/test/Transforms/OpenMP/lower-workshare-nowait.mlir
@@ -21,3 +21,50 @@ } return } + +// ----- + +// Check that nowait is not propagated into a region which is nested in +// something that is not itself the last piece of work of the omp.workshare +// region, or that may run more than once. + +// CHECK-LABEL: func.func @no_nowait_in_nested_conditional +func.func @no_nowait_in_nested_conditional(%arg0: !fir.ref<i32>, %cond: i1) { + omp.parallel { + omp.workshare { + %c1 = arith.constant 1 : index + %c10 = arith.constant 10 : index + fir.do_loop %i = %c1 to %c10 step %c1 { + fir.if %cond { + omp.workshare.loop_wrapper { + omp.loop_nest (%j) : index = (%c1) to (%c10) inclusive step (%c1) { + "test.inner"(%j) : (index) -> () + omp.yield + } + } + "test.side_effect"(%arg0) : (!fir.ref<i32>) -> () + } + } + "test.after_loop"(%arg0) : (!fir.ref<i32>) -> () + omp.terminator + } + omp.terminator + } + return +} + +// CHECK: fir.do_loop +// CHECK: fir.if +// CHECK: omp.wsloop { +// CHECK-NOT: nowait +// CHECK: omp.single { +// CHECK: "test.side_effect" +// CHECK: omp.terminator +// CHECK-NEXT: } +// The work after the loop is the last one, so it may use nowait and rely on +// the barrier at the end of the omp.workshare region. +// CHECK: omp.single nowait { +// CHECK: "test.after_loop" +// CHECK: omp.terminator +// CHECK-NEXT: } +// CHECK-NEXT: omp.barrier
diff --git a/test/Transforms/OpenMP/lower-workshare-thread-local.mlir b/test/Transforms/OpenMP/lower-workshare-thread-local.mlir index d6000c9..e88e205 100644 --- a/test/Transforms/OpenMP/lower-workshare-thread-local.mlir +++ b/test/Transforms/OpenMP/lower-workshare-thread-local.mlir
@@ -317,11 +317,14 @@ } // The store to thread-local memory is parallelized (outside the single), -// but the load remains inside the single to maintain synchronization. +// but the load remains inside the single to maintain synchronization. The +// store which depends on that load can only run on the thread executing the +// single, so the thread-local memory it updates is broadcast with copyprivate +// to keep the copies of the other threads in sync. // CHECK: omp.parallel { // CHECK-NEXT: %[[ALLOCA:.*]] = fir.alloca i32 -// CHECK: omp.single nowait { +// CHECK: omp.single copyprivate(%[[ALLOCA]] -> @_workshare_copy_i32 : !fir.ref<i32>) { // CHECK: fir.store {{.*}} to %[[ALLOCA]] : !fir.ref<i32> // CHECK: fir.load %[[ALLOCA]] : !fir.ref<i32> // CHECK: fir.store {{.*}} to %[[ALLOCA]] : !fir.ref<i32> @@ -403,3 +406,248 @@ // CHECK: } // CHECK: omp.barrier // CHECK: } + +// Check the FORALL fetch-counter pattern: a thread-local counter which is +// read, incremented and written back from inside an omp.single. +// +// !$omp workshare +// forall (i=1:1) +// forall (j=1:3) +// a(:,i,j) = a(:,i,j) + 1 +// end forall +// end forall +// !$omp end workshare +// +// The increment can only be computed on the thread executing the omp.single, +// so the counter must be broadcast with copyprivate. Otherwise the threads +// which did not execute the omp.single keep a stale counter and fetch the +// wrong element on the following iterations. See issue #209942. + +// CHECK-LABEL: func.func @forall_fetch_counter_in_workshare +func.func @forall_fetch_counter_in_workshare(%stack: !fir.ref<i32>) { + omp.parallel { + %counter = fir.alloca i64 {pinned} + omp.workshare { + %c0_i64 = arith.constant 0 : i64 + %c1_i64 = arith.constant 1 : i64 + %c1 = arith.constant 1 : index + %c3 = arith.constant 3 : index + fir.store %c0_i64 to %counter : !fir.ref<i64> + fir.do_loop %iv = %c1 to %c3 step %c1 { + %idx = fir.load %counter : !fir.ref<i64> + %next = arith.addi %idx, %c1_i64 : i64 + fir.store %next to %counter : !fir.ref<i64> + "test.fetch"(%stack, %idx) : (!fir.ref<i32>, i64) -> () + omp.workshare.loop_wrapper { + omp.loop_nest (%j) : index = (%c1) to (%c3) inclusive step (%c1) { + "test.inner"(%j) : (index) -> () + omp.yield + } + } + } + omp.terminator + } + omp.terminator + } + return +} + +// CHECK: omp.parallel { +// CHECK: %[[COUNTER:.*]] = fir.alloca i64 {pinned} +// The reset of the counter is a write to thread-local memory whose operands +// are all available, so it is parallelized and all threads run it. +// CHECK: fir.store %{{.*}} to %[[COUNTER]] : !fir.ref<i64> +// CHECK: fir.do_loop +// CHECK: omp.single copyprivate(%[[COUNTER]] -> @_workshare_copy_i64 : !fir.ref<i64>) { +// CHECK: %[[IDX:.*]] = fir.load %[[COUNTER]] : !fir.ref<i64> +// CHECK: %[[NEXT:.*]] = arith.addi %[[IDX]], %{{.*}} : i64 +// CHECK: fir.store %[[NEXT]] to %[[COUNTER]] : !fir.ref<i64> +// CHECK: "test.fetch" +// CHECK: omp.terminator +// CHECK-NEXT: } +// The increment must not be repeated outside the single. +// CHECK-NOT: fir.store {{.*}} to %[[COUNTER]] +// CHECK: omp.wsloop { + + +// ----- + +// Check that a thread-local location written from within an omp.single but +// never read back by the team is NOT broadcasted with copyprivate. The store +// is not safe to parallelize on its own here because its value comes from a +// shared load that must stay in the omp.single, so it ends up executed by a +// single thread. + +// CHECK-LABEL: func.func @write_only_thread_local_not_broadcast +func.func @write_only_thread_local_not_broadcast(%shared: !fir.ref<i32>) { + omp.parallel { + %tl = fir.alloca i32 + omp.workshare { + %v = fir.load %shared : !fir.ref<i32> + fir.store %v to %tl : !fir.ref<i32> + omp.terminator + } + omp.terminator + } + return +} + +// CHECK: omp.parallel { +// CHECK-NEXT: %[[TL:.*]] = fir.alloca i32 +// The single carries no copyprivate: %[[TL]] is never read by the team. +// CHECK: omp.single nowait { +// CHECK-NOT: copyprivate +// CHECK: fir.store %{{.*}} to %[[TL]] : !fir.ref<i32> +// CHECK: omp.terminator +// CHECK-NEXT: } +// CHECK-NEXT: omp.barrier + +// ----- + +// Same write from within an omp.single, but now the location is read back by +// the whole team after the omp.workshare region: it must be broadcasted so the +// threads which did not run the single do not observe a stale value. + +// CHECK-LABEL: func.func @written_then_read_thread_local_is_broadcast +func.func @written_then_read_thread_local_is_broadcast(%shared: !fir.ref<i32>, %sink: !fir.ref<i32>) { + omp.parallel { + %tl = fir.alloca i32 + omp.workshare { + %v = fir.load %shared : !fir.ref<i32> + fir.store %v to %tl : !fir.ref<i32> + omp.terminator + } + %r = fir.load %tl : !fir.ref<i32> + fir.store %r to %sink : !fir.ref<i32> + omp.terminator + } + return +} + +// CHECK: omp.parallel { +// CHECK-NEXT: %[[TL:.*]] = fir.alloca i32 +// CHECK: omp.single copyprivate(%[[TL]] -> @_workshare_copy_i32 : !fir.ref<i32>) { +// CHECK: fir.store %{{.*}} to %[[TL]] : !fir.ref<i32> +// CHECK: omp.terminator +// CHECK-NEXT: } +// CHECK: fir.load %[[TL]] : !fir.ref<i32> + + +// ----- + +// Check that the read which keeps a thread-local write live for broadcasting +// is still recognized when it goes through a fir.declare of the allocation +// (looking through declares/converts, as flang lowering routinely inserts +// them). Matching the raw load/store value instead would miss this read and +// drop the required broadcast. + +// CHECK-LABEL: func.func @broadcast_when_read_through_declare +func.func @broadcast_when_read_through_declare(%shared: !fir.ref<i32>, %sink: !fir.ref<i32>) { + omp.parallel { + %tl = fir.alloca i32 + %d = fir.declare %tl {uniq_name = "tl"} : (!fir.ref<i32>) -> !fir.ref<i32> + omp.workshare { + %v = fir.load %shared : !fir.ref<i32> + fir.store %v to %tl : !fir.ref<i32> + omp.terminator + } + %r = fir.load %d : !fir.ref<i32> + fir.store %r to %sink : !fir.ref<i32> + omp.terminator + } + return +} + +// CHECK: %[[TL:.*]] = fir.alloca i32 +// The broadcast copies the underlying allocation, not the fir.declare. +// CHECK: omp.single copyprivate(%[[TL]] -> @_workshare_copy_i32 : !fir.ref<i32>) { + +// ----- + +// Same, but now the write from within the omp.single goes through a +// fir.declare of the allocation while the read is direct. + +// CHECK-LABEL: func.func @broadcast_when_written_through_declare +func.func @broadcast_when_written_through_declare(%shared: !fir.ref<i32>, %sink: !fir.ref<i32>) { + omp.parallel { + %tl = fir.alloca i32 + %d = fir.declare %tl {uniq_name = "tl"} : (!fir.ref<i32>) -> !fir.ref<i32> + omp.workshare { + %v = fir.load %shared : !fir.ref<i32> + fir.store %v to %d : !fir.ref<i32> + omp.terminator + } + %r = fir.load %tl : !fir.ref<i32> + fir.store %r to %sink : !fir.ref<i32> + omp.terminator + } + return +} + +// CHECK: %[[TL:.*]] = fir.alloca i32 +// CHECK: omp.single copyprivate(%[[TL]] -> @_workshare_copy_i32 : !fir.ref<i32>) { + +// ----- + +// Check that a write performed from within an omp.single through an +// hlfir.declare of a privatizing clause block argument is still broadcasted +// when the location is read back through the block argument itself. The alias +// analysis reports the declare result for the write but the block argument for +// the read; getOpenMPThreadLocalSource canonicalizes both to the block +// argument so the broadcast is not dropped. + +omp.private {type = private} @z_private : i32 + +// CHECK-LABEL: func.func @broadcast_private_write_through_declare_read_direct +func.func @broadcast_private_write_through_declare_read_direct( + %arg0: !fir.ref<i32>, %shared: !fir.ref<i32>, %sink: !fir.ref<i32>) { + omp.parallel private(@z_private %arg0 -> %priv_arg : !fir.ref<i32>) { + %decl:2 = hlfir.declare %priv_arg {uniq_name = "z"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) + omp.workshare { + // The value comes from a shared load, so the store stays in the single. + %v = fir.load %shared : !fir.ref<i32> + fir.store %v to %decl#0 : !fir.ref<i32> + omp.terminator + } + // Read back the private variable directly through the block argument. + %r = fir.load %priv_arg : !fir.ref<i32> + fir.store %r to %sink : !fir.ref<i32> + omp.terminator + } + return +} + +// CHECK: omp.parallel private(@z_private %{{.*}} -> %[[PRIV:.*]] : !fir.ref<i32>) { +// The broadcast copies the private block argument, matching the read. +// CHECK: omp.single copyprivate(%[[PRIV]] -> @_workshare_copy_i32 : !fir.ref<i32>) { + +// ----- + +// Check that a thread-local location written from within an omp.single but +// never read back through a visible load is still broadcasted when the team may +// run an opaque call: the callee might read the location, and that read cannot +// be attributed to a specific location. Contrast with +// @write_only_thread_local_not_broadcast, which has no call and is left with a +// plain omp.single. + +func.func private @opaque(!fir.ref<i32>) + +// CHECK-LABEL: func.func @opaque_call_forces_broadcast +func.func @opaque_call_forces_broadcast(%shared: !fir.ref<i32>) { + omp.parallel { + %tl = fir.alloca i32 + omp.workshare { + %v = fir.load %shared : !fir.ref<i32> + fir.store %v to %tl : !fir.ref<i32> + omp.terminator + } + // The callee might read %tl, so the write must be broadcasted. + fir.call @opaque(%tl) : (!fir.ref<i32>) -> () + omp.terminator + } + return +} + +// CHECK: %[[TL:.*]] = fir.alloca i32 +// CHECK: omp.single copyprivate(%[[TL]] -> @_workshare_copy_i32 : !fir.ref<i32>) { +// CHECK: fir.store %{{.*}} to %[[TL]] : !fir.ref<i32>