[llvm-branch-commits] [flang] release/19.x: [flang][OpenMP] Fix copyprivate semantic checks (#95799) (PR #100352)
luporl wrote: Closing this, as it seems a new PR will be needed later, to merge https://github.com/llvm/llvm-project/pull/101009 instead. https://github.com/llvm/llvm-project/pull/100352 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] release/19.x: [flang][OpenMP] Fix copyprivate semantic checks (#95799) (PR #100352)
https://github.com/luporl closed https://github.com/llvm/llvm-project/pull/100352 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] release/19.x: [flang][OpenMP] Fix copyprivate semantic checks (#95799) (PR #100352)
luporl wrote: #95799 caused errors in some internal test suites and was reverted for now. https://github.com/llvm/llvm-project/pull/100352 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] release/19.x: [flang][OpenMP] Fix copyprivate semantic checks (#95799) (PR #100352)
https://github.com/luporl converted_to_draft https://github.com/llvm/llvm-project/pull/100352 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Support tasks' implicit firstprivate DSA (PR #85989)
@@ -2012,34 +2012,87 @@ void OmpAttributeVisitor::Post(const parser::Name ) { } } } -std::vector defaultDSASymbols; + +// Implicitly determined DSAs +// OMP 5.2 5.1.1 - Variables Referenced in a Construct +Symbol *lastDeclSymbol = nullptr; +std::optional prevDSA; for (int dirDepth{0}; dirDepth < (int)dirContext_.size(); ++dirDepth) { DirContext = dirContext_[dirDepth]; - bool hasDataSharingAttr{false}; + std::optional dsa; + for (auto symMap : dirContext.objectWithDSA) { // if the `symbol` already has a data-sharing attribute if (symMap.first->name() == name.symbol->name()) { - hasDataSharingAttr = true; + dsa = symMap.second; break; } } - if (hasDataSharingAttr) { -if (defaultDSASymbols.size()) - symbol = (symbol->name(), *defaultDSASymbols.back(), + + // When handling each implicit rule, either a new private symbol is + // declared or the last declared symbol is used. + // In the latter case, it's necessary to insert a new symbol in the scope + // being processed, associated with the last declared symbol, to avoid + // "inheriting" the enclosing context's symbol and its flags. luporl wrote: I have expanded the explanation of why a new symbol is needed when inheriting the enclosing context's symbol. Please check if it is clear now. https://github.com/llvm/llvm-project/pull/85989 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Support tasks' implicit firstprivate DSA (PR #85989)
https://github.com/luporl updated https://github.com/llvm/llvm-project/pull/85989 >From 94301e00239b789cf90d5291b1d733f0f2baab6c Mon Sep 17 00:00:00 2001 From: Leandro Lupori Date: Mon, 11 Mar 2024 16:47:47 -0300 Subject: [PATCH 1/2] [flang][OpenMP] Support tasks' implicit firstprivate DSA Handle implicit firstprivate DSAs on task generating constructs. Fixes https://github.com/llvm/llvm-project/issues/64480 --- flang/include/flang/Semantics/symbol.h| 3 +- .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 114 +++- flang/lib/Lower/OpenMP/DataSharingProcessor.h | 11 +- flang/lib/Semantics/resolve-directives.cpp| 85 +- .../test/Lower/OpenMP/FIR/default-clause.f90 | 3 +- .../Lower/OpenMP/default-clause-byref.f90 | 4 +- flang/test/Lower/OpenMP/default-clause.f90| 4 +- flang/test/Lower/OpenMP/implicit-dsa.f90 | 275 ++ flang/test/Semantics/OpenMP/implicit-dsa.f90 | 158 ++ flang/test/Semantics/OpenMP/symbol08.f90 | 2 +- 10 files changed, 617 insertions(+), 42 deletions(-) create mode 100644 flang/test/Lower/OpenMP/implicit-dsa.f90 create mode 100644 flang/test/Semantics/OpenMP/implicit-dsa.f90 diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h index 67153ffb3be9f6..34ac674614a695 100644 --- a/flang/include/flang/Semantics/symbol.h +++ b/flang/include/flang/Semantics/symbol.h @@ -740,7 +740,8 @@ class Symbol { OmpCommonBlock, OmpReduction, OmpAligned, OmpNontemporal, OmpAllocate, OmpDeclarativeAllocateDirective, OmpExecutableAllocateDirective, OmpDeclareSimd, OmpDeclareTarget, OmpThreadprivate, OmpDeclareReduction, - OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, OmpPreDetermined); + OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, OmpPreDetermined, + OmpImplicit); using Flags = common::EnumSet; const Scope () const { return *owner_; } diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp index 90c7e46dd183e3..792b3341ef03cb 100644 --- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp +++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp @@ -26,8 +26,10 @@ namespace omp { void DataSharingProcessor::processStep1() { collectSymbolsForPrivatization(); collectDefaultSymbols(); + collectImplicitSymbols(); privatize(); defaultPrivatize(); + implicitPrivatize(); insertBarrier(); } @@ -268,16 +270,94 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) { firOpBuilder.restoreInsertionPoint(localInsPt); } +static const Fortran::parser::CharBlock * +getSource(const Fortran::semantics::SemanticsContext , + const Fortran::lower::pft::Evaluation ) { + const Fortran::parser::CharBlock *source = nullptr; + + auto ompConsVisit = [&](const Fortran::parser::OpenMPConstruct ) { +std::visit(Fortran::common::visitors{ + [&](const Fortran::parser::OpenMPSectionsConstruct ) { + source = ::get<0>(x.t).source; + }, + [&](const Fortran::parser::OpenMPLoopConstruct ) { + source = ::get<0>(x.t).source; + }, + [&](const Fortran::parser::OpenMPBlockConstruct ) { + source = ::get<0>(x.t).source; + }, + [&](const Fortran::parser::OpenMPCriticalConstruct ) { + source = ::get<0>(x.t).source; + }, + [&](const Fortran::parser::OpenMPAtomicConstruct ) { + std::visit([&](const auto ) { source = }, +x.u); + }, + [&](const auto ) { source = }, + }, + x.u); + }; + + eval.visit(Fortran::common::visitors{ + [&](const Fortran::parser::OpenMPConstruct ) { ompConsVisit(x); }, + [&](const Fortran::parser::OpenMPDeclarativeConstruct ) { +source = + }, + [&](const Fortran::parser::OmpEndLoopDirective ) { +source = + }, + [&](const auto ) {}, + }); + + return source; +} + void DataSharingProcessor::collectSymbols( -Fortran::semantics::Symbol::Flag flag) { - converter.collectSymbolSet(eval, defaultSymbols, flag, +Fortran::semantics::Symbol::Flag flag, +llvm::SetVector ) { + // Collect all scopes associated with 'eval'. + llvm::SetVector clauseScopes; + std::function collectScopes = + [&](const Fortran::semantics::Scope *scope) { +clauseScopes.insert(scope); +for (const Fortran::semantics::Scope : scope->children()) + collectScopes(); + }; + const Fortran::parser::CharBlock *source = + clauses.empty() ? getSource(semaCtx, eval) : ().source; + const Fortran::semantics::Scope *curScope = nullptr; + if (source && !source->empty()) { +curScope = (*source); +collectScopes(curScope); + }
[llvm-branch-commits] [flang] [flang][OpenMP] Move clause/object conversion to happen early, in genOMP (PR #87086)
https://github.com/luporl approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/87086 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Support tasks' implicit firstprivate DSA (PR #85989)
luporl wrote: @NimishMishra Thanks for the review! It seems the buildbot errors on Windows are all of this type: `fatal error C1002: compiler is out of heap space in pass 2`. > Is this merge not in main? It is not. It depends on https://github.com/llvm/llvm-project/pull/85978, that depends on https://github.com/llvm/llvm-project/pull/72510. As tasks' implicit firstprivate happens only when no default clause is specified, I thought it would be better to start from the default clause fix, in https://github.com/llvm/llvm-project/pull/72510. Actually, https://github.com/llvm/llvm-project/pull/78283 is also needed to avoid issues with threadprivate. https://github.com/llvm/llvm-project/pull/85989 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Support tasks' implicit firstprivate DSA (PR #85989)
https://github.com/luporl created https://github.com/llvm/llvm-project/pull/85989 Handle implicit firstprivate DSAs on task generating constructs. Fixes https://github.com/llvm/llvm-project/issues/64480 >From 94301e00239b789cf90d5291b1d733f0f2baab6c Mon Sep 17 00:00:00 2001 From: Leandro Lupori Date: Mon, 11 Mar 2024 16:47:47 -0300 Subject: [PATCH] [flang][OpenMP] Support tasks' implicit firstprivate DSA Handle implicit firstprivate DSAs on task generating constructs. Fixes https://github.com/llvm/llvm-project/issues/64480 --- flang/include/flang/Semantics/symbol.h| 3 +- .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 114 +++- flang/lib/Lower/OpenMP/DataSharingProcessor.h | 11 +- flang/lib/Semantics/resolve-directives.cpp| 85 +- .../test/Lower/OpenMP/FIR/default-clause.f90 | 3 +- .../Lower/OpenMP/default-clause-byref.f90 | 4 +- flang/test/Lower/OpenMP/default-clause.f90| 4 +- flang/test/Lower/OpenMP/implicit-dsa.f90 | 275 ++ flang/test/Semantics/OpenMP/implicit-dsa.f90 | 158 ++ flang/test/Semantics/OpenMP/symbol08.f90 | 2 +- 10 files changed, 617 insertions(+), 42 deletions(-) create mode 100644 flang/test/Lower/OpenMP/implicit-dsa.f90 create mode 100644 flang/test/Semantics/OpenMP/implicit-dsa.f90 diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h index 67153ffb3be9f6..34ac674614a695 100644 --- a/flang/include/flang/Semantics/symbol.h +++ b/flang/include/flang/Semantics/symbol.h @@ -740,7 +740,8 @@ class Symbol { OmpCommonBlock, OmpReduction, OmpAligned, OmpNontemporal, OmpAllocate, OmpDeclarativeAllocateDirective, OmpExecutableAllocateDirective, OmpDeclareSimd, OmpDeclareTarget, OmpThreadprivate, OmpDeclareReduction, - OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, OmpPreDetermined); + OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, OmpPreDetermined, + OmpImplicit); using Flags = common::EnumSet; const Scope () const { return *owner_; } diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp index 90c7e46dd183e3..792b3341ef03cb 100644 --- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp +++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp @@ -26,8 +26,10 @@ namespace omp { void DataSharingProcessor::processStep1() { collectSymbolsForPrivatization(); collectDefaultSymbols(); + collectImplicitSymbols(); privatize(); defaultPrivatize(); + implicitPrivatize(); insertBarrier(); } @@ -268,16 +270,94 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) { firOpBuilder.restoreInsertionPoint(localInsPt); } +static const Fortran::parser::CharBlock * +getSource(const Fortran::semantics::SemanticsContext , + const Fortran::lower::pft::Evaluation ) { + const Fortran::parser::CharBlock *source = nullptr; + + auto ompConsVisit = [&](const Fortran::parser::OpenMPConstruct ) { +std::visit(Fortran::common::visitors{ + [&](const Fortran::parser::OpenMPSectionsConstruct ) { + source = ::get<0>(x.t).source; + }, + [&](const Fortran::parser::OpenMPLoopConstruct ) { + source = ::get<0>(x.t).source; + }, + [&](const Fortran::parser::OpenMPBlockConstruct ) { + source = ::get<0>(x.t).source; + }, + [&](const Fortran::parser::OpenMPCriticalConstruct ) { + source = ::get<0>(x.t).source; + }, + [&](const Fortran::parser::OpenMPAtomicConstruct ) { + std::visit([&](const auto ) { source = }, +x.u); + }, + [&](const auto ) { source = }, + }, + x.u); + }; + + eval.visit(Fortran::common::visitors{ + [&](const Fortran::parser::OpenMPConstruct ) { ompConsVisit(x); }, + [&](const Fortran::parser::OpenMPDeclarativeConstruct ) { +source = + }, + [&](const Fortran::parser::OmpEndLoopDirective ) { +source = + }, + [&](const auto ) {}, + }); + + return source; +} + void DataSharingProcessor::collectSymbols( -Fortran::semantics::Symbol::Flag flag) { - converter.collectSymbolSet(eval, defaultSymbols, flag, +Fortran::semantics::Symbol::Flag flag, +llvm::SetVector ) { + // Collect all scopes associated with 'eval'. + llvm::SetVector clauseScopes; + std::function collectScopes = + [&](const Fortran::semantics::Scope *scope) { +clauseScopes.insert(scope); +for (const Fortran::semantics::Scope : scope->children()) + collectScopes(); + }; + const Fortran::parser::CharBlock *source = + clauses.empty() ? getSource(semaCtx, eval) : ().source; + const Fortran::semantics::Scope
[llvm-branch-commits] [flang] [flang][OpenMP] Refactor nested default clause tests (PR #85978)
luporl wrote: This PR depends on https://github.com/llvm/llvm-project/pull/72510. https://github.com/llvm/llvm-project/pull/85978 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [flang] [flang][OpenMP] Refactor nested default clause tests (PR #85978)
https://github.com/luporl created https://github.com/llvm/llvm-project/pull/85978 Split nested default clause tests into multiple subroutines, to make it easier to find failures. While here, fix indentation of the modified lines. >From 39b3e13e42cd9416101ddb7b5cbb4137e5bd66f3 Mon Sep 17 00:00:00 2001 From: Leandro Lupori Date: Wed, 20 Mar 2024 14:16:10 -0300 Subject: [PATCH] [flang][OpenMP] Refactor nested default clause tests Split nested default clause tests into multiple subroutines, to make it easier to find failures. While here, fix indentation of the modified lines. --- .../Lower/OpenMP/default-clause-byref.f90 | 6 +- flang/test/Lower/OpenMP/default-clause.f90| 273 ++ 2 files changed, 155 insertions(+), 124 deletions(-) diff --git a/flang/test/Lower/OpenMP/default-clause-byref.f90 b/flang/test/Lower/OpenMP/default-clause-byref.f90 index 5d9538e53069d6..86da354910a8ef 100644 --- a/flang/test/Lower/OpenMP/default-clause-byref.f90 +++ b/flang/test/Lower/OpenMP/default-clause-byref.f90 @@ -226,8 +226,6 @@ subroutine nested_default_clause_tests !CHECK: %[[PRIVATE_Y_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Y]] {uniq_name = "_QFnested_default_clause_testsEy"} : (!fir.ref) -> (!fir.ref, !fir.ref) !CHECK: %[[PRIVATE_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name = "_QFnested_default_clause_testsEz"} !CHECK: %[[PRIVATE_Z_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_Z]] {uniq_name = "_QFnested_default_clause_testsEz"} : (!fir.ref) -> (!fir.ref, !fir.ref) -!CHECK: %[[PRIVATE_W:.*]] = fir.alloca i32 {bindc_name = "w", pinned, uniq_name = "_QFnested_default_clause_testsEw"} -!CHECK: %[[PRIVATE_W_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_W]] {uniq_name = "_QFnested_default_clause_testsEw"} : (!fir.ref) -> (!fir.ref, !fir.ref) !CHECK: omp.parallel { !CHECK: %[[PRIVATE_INNER_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFnested_default_clause_testsEx"} !CHECK: %[[PRIVATE_INNER_X_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_X]] {uniq_name = "_QFnested_default_clause_testsEx"} : (!fir.ref) -> (!fir.ref, !fir.ref) @@ -242,12 +240,14 @@ subroutine nested_default_clause_tests !CHECK: omp.terminator !CHECK: } !CHECK: omp.parallel { +!CHECK: %[[PRIVATE_INNER_Z:.*]] = fir.alloca i32 {bindc_name = "z", pinned, uniq_name = "_QFnested_default_clause_testsEz"} +!CHECK: %[[PRIVATE_INNER_Z_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_Z]] {uniq_name = "_QFnested_default_clause_testsEz"} : (!fir.ref) -> (!fir.ref, !fir.ref) !CHECK: %[[PRIVATE_INNER_W:.*]] = fir.alloca i32 {bindc_name = "w", pinned, uniq_name = "_QFnested_default_clause_testsEw"} !CHECK: %[[PRIVATE_INNER_W_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_W]] {uniq_name = "_QFnested_default_clause_testsEw"} : (!fir.ref) -> (!fir.ref, !fir.ref) !CHECK: %[[PRIVATE_INNER_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFnested_default_clause_testsEx"} !CHECK: %[[PRIVATE_INNER_X_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_INNER_X]] {uniq_name = "_QFnested_default_clause_testsEx"} : (!fir.ref) -> (!fir.ref, !fir.ref) !CHECK: %[[TEMP_1:.*]] = fir.load %[[PRIVATE_INNER_X_DECL]]#0 : !fir.ref -!CHECK: %[[TEMP_2:.*]] = fir.load %[[PRIVATE_Z_DECL]]#0 : !fir.ref +!CHECK: %[[TEMP_2:.*]] = fir.load %[[PRIVATE_INNER_Z_DECL]]#0 : !fir.ref !CHECK: %[[RESULT:.*]] = arith.addi %{{.*}}, %{{.*}} : i32 !CHECK: hlfir.assign %[[RESULT]] to %[[PRIVATE_INNER_W_DECL]]#0 : i32, !fir.ref !CHECK: omp.terminator diff --git a/flang/test/Lower/OpenMP/default-clause.f90 b/flang/test/Lower/OpenMP/default-clause.f90 index 6f949785876f66..f69b5e607d3561 100644 --- a/flang/test/Lower/OpenMP/default-clause.f90 +++ b/flang/test/Lower/OpenMP/default-clause.f90 @@ -148,34 +148,33 @@ program default_clause_lowering end program default_clause_lowering -subroutine nested_default_clause_tests -integer :: x, y, z, w, k, a -!CHECK: %[[K:.*]] = fir.alloca i32 {bindc_name = "k", uniq_name = "_QFnested_default_clause_testsEk"} -!CHECK: %[[K_DECL:.*]]:2 = hlfir.declare %[[K]] {uniq_name = "_QFnested_default_clause_testsEk"} : (!fir.ref) -> (!fir.ref, !fir.ref) -!CHECK: %[[W:.*]] = fir.alloca i32 {bindc_name = "w", uniq_name = "_QFnested_default_clause_testsEw"} -!CHECK: %[[W_DECL:.*]]:2 = hlfir.declare %[[W]] {uniq_name = "_QFnested_default_clause_testsEw"} : (!fir.ref) -> (!fir.ref, !fir.ref) -!CHECK: %[[X:.*]] = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFnested_default_clause_testsEx"} -!CHECK: %[[X_DECL:.*]]:2 = hlfir.declare %[[X]] {uniq_name = "_QFnested_default_clause_testsEx"} : (!fir.ref) -> (!fir.ref, !fir.ref) -!CHECK: %[[Y:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFnested_default_clause_testsEy"} -!CHECK: %[[Y_DECL:.*]]:2 = hlfir.declare %[[Y]] {uniq_name = "_QFnested_default_clause_testsEy"} : (!fir.ref) -> (!fir.ref, !fir.ref) -!CHECK: %[[Z:.*]] = fir.alloca i32 {bindc_name = "z", uniq_name = "_QFnested_default_clause_testsEz"} -!CHECK: %[[Z_DECL:.*]]:2 =
[llvm-branch-commits] [flang] [flang][OpenMP] Add support for copyprivate (PR #80485)
@@ -1092,6 +1040,79 @@ class FirConverter : public Fortran::lower::AbstractConverter { return true; } + void copyVar(const Fortran::semantics::Symbol , + const Fortran::lower::SymbolBox _sb, + const Fortran::lower::SymbolBox _sb) { +mlir::Location loc = genLocation(sym.name()); +if (lowerToHighLevelFIR()) + copyVarHLFIR(loc, lhs_sb.getAddr(), rhs_sb.getAddr()); +else + copyVarFIR(loc, sym, lhs_sb, rhs_sb); + } + + void copyVarHLFIR(mlir::Location loc, mlir::Value dst, mlir::Value src) { +assert(lowerToHighLevelFIR()); +hlfir::Entity lhs{dst}; +hlfir::Entity rhs{src}; +// Temporary_lhs is set to true in hlfir.assign below to avoid user +// assignment to be used and finalization to be called on the LHS. +// This may or may not be correct but mimics the current behaviour +// without HLFIR. +auto copyData = [&](hlfir::Entity l, hlfir::Entity r) { + // Dereference RHS and load it if trivial scalar. + r = hlfir::loadTrivialScalar(loc, *builder, r); + builder->create( + loc, r, l, + /*isWholeAllocatableAssignment=*/false, + /*keepLhsLengthInAllocatableAssignment=*/false, + /*temporary_lhs=*/true); +}; +if (lhs.isAllocatable()) { + // Deep copy allocatable if it is allocated. + // Note that when allocated, the RHS is already allocated with the LHS + // shape for copy on entry in createHostAssociateVarClone. + // For lastprivate, this assumes that the RHS was not reallocated in + // the OpenMP region. + lhs = hlfir::derefPointersAndAllocatables(loc, *builder, lhs); + mlir::Value addr = hlfir::genVariableRawAddress(loc, *builder, lhs); + mlir::Value isAllocated = builder->genIsNotNullAddr(loc, addr); + builder->genIfThen(loc, isAllocated) + .genThen([&]() { +// Copy the DATA, not the descriptors. +copyData(lhs, rhs); + }) + .end(); +} else if (lhs.isPointer()) { + // Set LHS target to the target of RHS (do not copy the RHS + // target data into the LHS target storage). + auto loadVal = builder->create(loc, rhs); + builder->create(loc, loadVal, lhs); +} else { + // Non ALLOCATABLE/POINTER variable. Simple DATA copy. + copyData(lhs, rhs); +} + } + + void copyVarFIR(mlir::Location loc, const Fortran::semantics::Symbol , + const Fortran::lower::SymbolBox _sb, + const Fortran::lower::SymbolBox _sb) { +assert(!lowerToHighLevelFIR()); +fir::ExtendedValue lhs = symBoxToExtendedValue(lhs_sb); +fir::ExtendedValue rhs = symBoxToExtendedValue(rhs_sb); +mlir::Type symType = genType(sym); +if (auto seqTy = symType.dyn_cast()) { + Fortran::lower::StatementContext stmtCtx; + Fortran::lower::createSomeArrayAssignment(*this, lhs, rhs, localSymbols, +stmtCtx); + stmtCtx.finalizeAndReset(); +} else if (lhs.getBoxOf()) { + fir::factory::CharacterExprHelper{*builder, loc}.createAssign(lhs, rhs); +} else { + auto loadVal = builder->create(loc, fir::getBase(rhs)); + builder->create(loc, loadVal, fir::getBase(lhs)); +} + } + luporl wrote: I guess it would be possible to move this to OpenMP.cpp, but this would mean duplicating around 40 lines of code. The `copyVarHLFIR()` code was extracted from `copyHostAssociateVar()`, that now calls `copyVar()` instead. Can we keep it in the converter to avoid code duplication? https://github.com/llvm/llvm-project/pull/80485 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [mlir] [llvm] [llvm][mlir][OMPIRBuilder] Translate omp.single's copyprivate (PR #80488)
https://github.com/luporl created https://github.com/llvm/llvm-project/pull/80488 Use the new copyprivate list from omp.single to emit calls to __kmpc_copyprivate, during the creation of the single operation in OMPIRBuilder. This is patch 4 of 4, to add support for COPYPRIVATE in Flang. Original PR: https://github.com/llvm/llvm-project/pull/73128 >From 52462e6790194c19465f017b81e51e4a99136d3a Mon Sep 17 00:00:00 2001 From: Leandro Lupori Date: Fri, 2 Feb 2024 17:16:34 -0300 Subject: [PATCH] [llvm][mlir][OMPIRBuilder] Translate omp.single's copyprivate Use the new copyprivate list from omp.single to emit calls to __kmpc_copyprivate, during the creation of the single operation in OMPIRBuilder. This is patch 4 of 4, to add support for COPYPRIVATE in Flang. Original PR: https://github.com/llvm/llvm-project/pull/73128 --- .../llvm/Frontend/OpenMP/OMPIRBuilder.h | 6 +- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 23 +++- .../Frontend/OpenMPIRBuilderTest.cpp | 111 ++ .../OpenMP/OpenMPToLLVMIRTranslation.cpp | 20 +++- mlir/test/Target/LLVMIR/openmp-llvm.mlir | 32 + 5 files changed, 187 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h index 669104307fa0e..ab92c172c75ae 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h @@ -1819,12 +1819,16 @@ class OpenMPIRBuilder { /// \param FiniCB Callback to finalize variable copies. /// \param IsNowait If false, a barrier is emitted. /// \param DidIt Local variable used as a flag to indicate 'single' thread + /// \param CPVars copyprivate variables. + /// \param CPFuncs copy functions to use for each copyprivate variable. /// /// \returns The insertion position *after* the single call. InsertPointTy createSingle(const LocationDescription , BodyGenCallbackTy BodyGenCB, FinalizeCallbackTy FiniCB, bool IsNowait, - llvm::Value *DidIt); + llvm::Value *DidIt, + ArrayRef CPVars = {}, + ArrayRef CPFuncs = {}); /// Generator for '#omp master' /// diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp index f6cf358119fb7..7abac0f660ef8 100644 --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp @@ -3992,7 +3992,8 @@ OpenMPIRBuilder::createCopyPrivate(const LocationDescription , OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createSingle( const LocationDescription , BodyGenCallbackTy BodyGenCB, -FinalizeCallbackTy FiniCB, bool IsNowait, llvm::Value *DidIt) { +FinalizeCallbackTy FiniCB, bool IsNowait, llvm::Value *DidIt, +ArrayRef CPVars, ArrayRef CPFuncs) { if (!updateToLocation(Loc)) return Loc.IP; @@ -4015,17 +4016,33 @@ OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createSingle( Function *ExitRTLFn = getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_end_single); Instruction *ExitCall = Builder.CreateCall(ExitRTLFn, Args); + auto FiniCBWrapper = [&](InsertPointTy IP) { +FiniCB(IP); + +if (DidIt) + Builder.CreateStore(Builder.getInt32(1), DidIt); + }; + // generates the following: // if (__kmpc_single()) { // single region ... // __kmpc_end_single // } + // __kmpc_copyprivate // __kmpc_barrier - EmitOMPInlinedRegion(OMPD, EntryCall, ExitCall, BodyGenCB, FiniCB, + EmitOMPInlinedRegion(OMPD, EntryCall, ExitCall, BodyGenCB, FiniCBWrapper, /*Conditional*/ true, /*hasFinalize*/ true); - if (!IsNowait) + + if (DidIt) { +for (size_t I = 0, E = CPVars.size(); I < E; ++I) + // NOTE BufSize is currently unused, so just pass 0. + createCopyPrivate(LocationDescription(Builder.saveIP(), Loc.DL), +/*BufSize=*/ConstantInt::get(Int64, 0), CPVars[I], +CPFuncs[I], DidIt); +// NOTE __kmpc_copyprivate already inserts a barrier + } else if (!IsNowait) createBarrier(LocationDescription(Builder.saveIP(), Loc.DL), omp::Directive::OMPD_unknown, /* ForceSimpleCall */ false, /* CheckCancelFlag */ false); diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp index e79d0bb2f65ae..0eb1039aa442c 100644 --- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp +++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp @@ -3464,6 +3464,117 @@ TEST_F(OpenMPIRBuilderTest, SingleDirectiveNowait) { EXPECT_EQ(ExitBarrier, nullptr); } +TEST_F(OpenMPIRBuilderTest, SingleDirectiveCopyPrivate) { + using InsertPointTy = OpenMPIRBuilder::InsertPointTy; + OpenMPIRBuilder OMPBuilder(*M); +
[llvm-branch-commits] [flang] [flang][OpenMP] Add support for copyprivate (PR #80485)
https://github.com/luporl created https://github.com/llvm/llvm-project/pull/80485 Add initial handling of OpenMP copyprivate clause in Flang. When lowering copyprivate, Flang generates the copy function needed by each variable and builds the appropriate omp.single's CopyPrivateVarList. This is patch 3 of 4, to add support for COPYPRIVATE in Flang. Original PR: https://github.com/llvm/llvm-project/pull/73128 >From 06f0aa95ccee98046f71a075610be0ed92849534 Mon Sep 17 00:00:00 2001 From: Leandro Lupori Date: Fri, 2 Feb 2024 16:31:20 -0300 Subject: [PATCH] [flang][OpenMP] Add support for copyprivate Add initial handling of OpenMP copyprivate clause in Flang. When lowering copyprivate, Flang generates the copy function needed by each variable and builds the appropriate omp.single's CopyPrivateVarList. This is patch 3 of 4, to add support for COPYPRIVATE in Flang. Original PR: https://github.com/llvm/llvm-project/pull/73128 --- flang/include/flang/Lower/AbstractConverter.h | 3 + flang/lib/Lower/Bridge.cpp| 137 -- flang/lib/Lower/OpenMP.cpp| 173 +- flang/test/Lower/OpenMP/Todo/copyprivate.f90 | 13 -- flang/test/Lower/OpenMP/copyprivate.f90 | 164 + 5 files changed, 414 insertions(+), 76 deletions(-) delete mode 100644 flang/test/Lower/OpenMP/Todo/copyprivate.f90 create mode 100644 flang/test/Lower/OpenMP/copyprivate.f90 diff --git a/flang/include/flang/Lower/AbstractConverter.h b/flang/include/flang/Lower/AbstractConverter.h index c19dcbdcdb390..48804c327e1c7 100644 --- a/flang/include/flang/Lower/AbstractConverter.h +++ b/flang/include/flang/Lower/AbstractConverter.h @@ -120,6 +120,9 @@ class AbstractConverter { const Fortran::semantics::Symbol , mlir::OpBuilder::InsertPoint *copyAssignIP = nullptr) = 0; + virtual void copyVar(mlir::Location loc, mlir::Value dst, + mlir::Value src) = 0; + /// For a given symbol, check if it is present in the inner-most /// level of the symbol map. virtual bool isPresentShallowLookup(Fortran::semantics::Symbol ) = 0; diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp index 8006b9b426f4d..04d09c4848491 100644 --- a/flang/lib/Lower/Bridge.cpp +++ b/flang/lib/Lower/Bridge.cpp @@ -743,6 +743,11 @@ class FirConverter : public Fortran::lower::AbstractConverter { }); } + void copyVar(mlir::Location loc, mlir::Value dst, + mlir::Value src) override final { +copyVarHLFIR(loc, dst, src); + } + void copyHostAssociateVar( const Fortran::semantics::Symbol , mlir::OpBuilder::InsertPoint *copyAssignIP = nullptr) override final { @@ -777,64 +782,7 @@ class FirConverter : public Fortran::lower::AbstractConverter { rhs_sb = } -mlir::Location loc = genLocation(sym.name()); - -if (lowerToHighLevelFIR()) { - hlfir::Entity lhs{lhs_sb->getAddr()}; - hlfir::Entity rhs{rhs_sb->getAddr()}; - // Temporary_lhs is set to true in hlfir.assign below to avoid user - // assignment to be used and finalization to be called on the LHS. - // This may or may not be correct but mimics the current behaviour - // without HLFIR. - auto copyData = [&](hlfir::Entity l, hlfir::Entity r) { -// Dereference RHS and load it if trivial scalar. -r = hlfir::loadTrivialScalar(loc, *builder, r); -builder->create( -loc, r, l, -/*isWholeAllocatableAssignment=*/false, -/*keepLhsLengthInAllocatableAssignment=*/false, -/*temporary_lhs=*/true); - }; - if (lhs.isAllocatable()) { -// Deep copy allocatable if it is allocated. -// Note that when allocated, the RHS is already allocated with the LHS -// shape for copy on entry in createHostAssociateVarClone. -// For lastprivate, this assumes that the RHS was not reallocated in -// the OpenMP region. -lhs = hlfir::derefPointersAndAllocatables(loc, *builder, lhs); -mlir::Value addr = hlfir::genVariableRawAddress(loc, *builder, lhs); -mlir::Value isAllocated = builder->genIsNotNullAddr(loc, addr); -builder->genIfThen(loc, isAllocated) -.genThen([&]() { - // Copy the DATA, not the descriptors. - copyData(lhs, rhs); -}) -.end(); - } else if (lhs.isPointer()) { -// Set LHS target to the target of RHS (do not copy the RHS -// target data into the LHS target storage). -auto loadVal = builder->create(loc, rhs); -builder->create(loc, loadVal, lhs); - } else { -// Non ALLOCATABLE/POINTER variable. Simple DATA copy. -copyData(lhs, rhs); - } -} else { - fir::ExtendedValue lhs = symBoxToExtendedValue(*lhs_sb); - fir::ExtendedValue rhs = symBoxToExtendedValue(*rhs_sb); - mlir::Type symType = genType(sym); - if (auto seqTy =
[llvm-branch-commits] [compiler-rt] bd6783b - [compiler-rt] Fix invalid triple on ARM build
Author: Leandro Lupori Date: 2023-04-27T11:10:43Z New Revision: bd6783b380768bd35f37e0034dccf6c5736dd031 URL: https://github.com/llvm/llvm-project/commit/bd6783b380768bd35f37e0034dccf6c5736dd031 DIFF: https://github.com/llvm/llvm-project/commit/bd6783b380768bd35f37e0034dccf6c5736dd031.diff LOG: [compiler-rt] Fix invalid triple on ARM build The fuzzer build was failing on armv7l, with an invalid triple error. This happened because CMake's get_compiler_rt_target function was missing some code to correctly handle arm archs, such as armhf. This was originaly part of https://reviews.llvm.org/D140011, that landed on main with commit cd173cbd7cca69c29df42cd4b42e60433435c29b. Fixes #60115 Differential Revision: https://reviews.llvm.org/D142906 Added: Modified: compiler-rt/cmake/Modules/CompilerRTUtils.cmake Removed: diff --git a/compiler-rt/cmake/Modules/CompilerRTUtils.cmake b/compiler-rt/cmake/Modules/CompilerRTUtils.cmake index 4c85551d77662..eefc466a46103 100644 --- a/compiler-rt/cmake/Modules/CompilerRTUtils.cmake +++ b/compiler-rt/cmake/Modules/CompilerRTUtils.cmake @@ -433,6 +433,25 @@ function(get_compiler_rt_target arch variable) string(REGEX REPLACE "mipsisa64" "mipsisa32" triple_cpu_mips "${triple_cpu}") string(REGEX REPLACE "mips64" "mips" triple_cpu_mips "${triple_cpu_mips}") set(target "${triple_cpu_mips}${triple_suffix_gnu}") + elseif("${arch}" MATCHES "^arm") +# Arch is arm, armhf, armv6m (anything else would come from using +# COMPILER_RT_DEFAULT_TARGET_ONLY, which is checked above). +if (${arch} STREQUAL "armhf") + # If we are building for hard float but our ABI is soft float. + if ("${triple_suffix}" MATCHES ".*eabi$") +# Change "eabi" -> "eabihf" +set(triple_suffix "${triple_suffix}hf") + endif() + # ABI is already set in the triple, don't repeat it in the architecture. + set(arch "arm") +else () + # If we are building for soft float, but the triple's ABI is hard float. + if ("${triple_suffix}" MATCHES ".*eabihf$") +# Change "eabihf" -> "eabi" +string(REGEX REPLACE "hf$" "" triple_suffix "${triple_suffix}") + endif() +endif() +set(target "${arch}${triple_suffix}") else() set(target "${arch}${triple_suffix}") endif() ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [compiler-rt] ccbab59 - [compiler-rt] Fix scudo build on ARM
Author: Leandro Lupori Date: 2023-04-27T11:10:43Z New Revision: ccbab5979b7bd594314b50f4fc54ec57a676f391 URL: https://github.com/llvm/llvm-project/commit/ccbab5979b7bd594314b50f4fc54ec57a676f391 DIFF: https://github.com/llvm/llvm-project/commit/ccbab5979b7bd594314b50f4fc54ec57a676f391.diff LOG: [compiler-rt] Fix scudo build on ARM The build of scudo was failing on armv7l, with undefined references to unwinder symbols, such as __aeabi_unwind_cpp_pr0. These are needed by RTGwpAsan and thus, on ARM, scudo must also be linked against an unwind library. The cmake command that caused the build failure was: cmake --fresh -S "$PWD/llvm/" -B "$PWD/build/" -G Ninja \ -DCMAKE_INSTALL_PREFIX="$PWD/install" \ -DCMAKE_BUILD_TYPE=Release \ -DLLVM_ENABLE_PROJECTS="clang;lld;lldb;clang-tools-extra;polly" \ -DLLVM_ENABLE_RUNTIMES="compiler-rt;libcxx;libcxxabi;libunwind" \ -DLLVM_TOOLCHAIN_TOOLS="llvm-ar;llvm-ranlib;llvm-objdump;\ llvm-rc;llvm-cvtres;llvm-nm;llvm-strings;llvm-readobj;\ llvm-dlltool;llvm-pdbutil;llvm-objcopy;llvm-strip;llvm-cov;\ llvm-profdata;llvm-addr2line;llvm-symbolizer;llvm-windres;llvm-ml;\ llvm-readelf;llvm-size" \ -DLLVM_INSTALL_BINUTILS_SYMLINKS=OFF -DLLVM_PARALLEL_LINK_JOBS=1 Fixes #60115 Reviewed By: hctim Differential Revision: https://reviews.llvm.org/D142888 (cherry picked from commit e1e972689b9138db795885a5468a15aafbe7cb51) Added: Modified: compiler-rt/lib/scudo/standalone/CMakeLists.txt Removed: diff --git a/compiler-rt/lib/scudo/standalone/CMakeLists.txt b/compiler-rt/lib/scudo/standalone/CMakeLists.txt index 9ac3340877da1..d75b7fd235230 100644 --- a/compiler-rt/lib/scudo/standalone/CMakeLists.txt +++ b/compiler-rt/lib/scudo/standalone/CMakeLists.txt @@ -129,8 +129,19 @@ set(SCUDO_SOURCES_CXX_WRAPPERS ) set(SCUDO_OBJECT_LIBS) +set(SCUDO_LINK_LIBS) if (COMPILER_RT_HAS_GWP_ASAN) + if(COMPILER_RT_USE_LLVM_UNWINDER) +list(APPEND SCUDO_LINK_LIBS ${COMPILER_RT_UNWINDER_LINK_LIBS} dl) + elseif (COMPILER_RT_HAS_GCC_S_LIB) +list(APPEND SCUDO_LINK_LIBS gcc_s) + elseif (COMPILER_RT_HAS_GCC_LIB) +list(APPEND SCUDO_LINK_LIBS gcc) + elseif (NOT COMPILER_RT_USE_BUILTINS_LIBRARY) +message(FATAL_ERROR "No suitable unwinder library") + endif() + add_dependencies(scudo_standalone gwp_asan) list(APPEND SCUDO_OBJECT_LIBS RTGwpAsan RTGwpAsanBacktraceLibc RTGwpAsanSegvHandler @@ -143,8 +154,6 @@ if (COMPILER_RT_HAS_GWP_ASAN) endif() -set(SCUDO_LINK_LIBS ${COMPILER_RT_UNWINDER_LINK_LIBS}) - if(COMPILER_RT_BUILD_SCUDO_STANDALONE_WITH_LLVM_LIBC) include_directories(${COMPILER_RT_BINARY_DIR}/../libc/include/) ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits