[llvm-branch-commits] [flang] [flang][OpenMP] Support tasks' implicit firstprivate DSA (PR #85989)

2024-04-24 Thread Leandro Lupori via llvm-branch-commits


@@ -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)

2024-04-24 Thread Leandro Lupori via llvm-branch-commits

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)

2024-04-01 Thread Leandro Lupori via llvm-branch-commits

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)

2024-03-26 Thread Leandro Lupori via llvm-branch-commits

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)

2024-03-20 Thread Leandro Lupori via llvm-branch-commits

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)

2024-03-20 Thread Leandro Lupori via llvm-branch-commits

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)

2024-03-20 Thread Leandro Lupori via llvm-branch-commits

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)

2024-02-05 Thread Leandro Lupori via llvm-branch-commits


@@ -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)

2024-02-02 Thread Leandro Lupori via llvm-branch-commits

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)

2024-02-02 Thread Leandro Lupori via llvm-branch-commits

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

2023-04-27 Thread Leandro Lupori via llvm-branch-commits

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

2023-04-27 Thread Leandro Lupori via llvm-branch-commits

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