[flang-commits] [flang] [flang][OpenMP] Move handling of OpenMP symbol flags to OpenMP.cpp (PR #75523)

Krzysztof Parzyszek via flang-commits flang-commits at lists.llvm.org
Fri Dec 15 06:40:14 PST 2023


https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/75523

>From 21261fef67dbbea956adf2e09b8abacd92d9caf3 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 14 Dec 2023 13:45:17 -0600
Subject: [PATCH 1/3] [flang][OpenMP] Move handling of OpenMP symbol flags to
 OpenMP.cpp

The function `instantiateVariable` in Bridge.cpp has the following code:
```
  if (var.getSymbol().test(
          Fortran::semantics::Symbol::Flag::OmpThreadprivate))
    Fortran::lower::genThreadprivateOp(*this, var);

  if (var.getSymbol().test(
          Fortran::semantics::Symbol::Flag::OmpDeclareTarget))
    Fortran::lower::genDeclareTargetIntGlobal(*this, var);
```

Implement `handleOpenMPSymbolProperties` in OpenMP.cpp, move the above
code there, and have `instantiateVariable` call this function instead.

This would further separate OpenMP-related details into OpenMP.cpp.
---
 flang/include/flang/Lower/OpenMP.h |  2 ++
 flang/lib/Lower/Bridge.cpp         |  8 +-------
 flang/lib/Lower/OpenMP.cpp         | 13 +++++++++++++
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/flang/include/flang/Lower/OpenMP.h b/flang/include/flang/Lower/OpenMP.h
index c9162761a08d54..a6ea26ee949fe7 100644
--- a/flang/include/flang/Lower/OpenMP.h
+++ b/flang/include/flang/Lower/OpenMP.h
@@ -56,6 +56,8 @@ void genOpenMPConstruct(AbstractConverter &, semantics::SemanticsContext &,
                         pft::Evaluation &, const parser::OpenMPConstruct &);
 void genOpenMPDeclarativeConstruct(AbstractConverter &, pft::Evaluation &,
                                    const parser::OpenMPDeclarativeConstruct &);
+void handleOpenMPSymbolProperties(AbstractConverter &converter,
+                                  const pft::Variable &var);
 int64_t getCollapseValue(const Fortran::parser::OmpClauseList &clauseList);
 void genThreadprivateOp(AbstractConverter &, const pft::Variable &);
 void genDeclareTargetIntGlobal(AbstractConverter &, const pft::Variable &);
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 6ca910d2696742..1ec242fafc5b6d 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -4244,13 +4244,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
                       Fortran::lower::AggregateStoreMap &storeMap) {
     Fortran::lower::instantiateVariable(*this, var, localSymbols, storeMap);
     if (var.hasSymbol()) {
-      if (var.getSymbol().test(
-              Fortran::semantics::Symbol::Flag::OmpThreadprivate))
-        Fortran::lower::genThreadprivateOp(*this, var);
-
-      if (var.getSymbol().test(
-              Fortran::semantics::Symbol::Flag::OmpDeclareTarget))
-        Fortran::lower::genDeclareTargetIntGlobal(*this, var);
+      handleOpenMPSymbolProperties(*this, var);
     }
   }
 
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 12b8ea82884d9d..9a798e641d1267 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -3483,6 +3483,19 @@ void Fortran::lower::genOpenMPDeclarativeConstruct(
       ompDeclConstruct.u);
 }
 
+void Fortran::lower::handleOpenMPSymbolProperties(
+    Fortran::lower::AbstractConverter &converter,
+    const Fortran::lower::pft::Variable &var) {
+  assert(var.hasSymbol() && "Expecting Symbol");
+  const Fortran::semantics::Symbol &sym = var.getSymbol();
+
+  if (sym.test(Fortran::semantics::Symbol::Flag::OmpThreadprivate))
+    Fortran::lower::genThreadprivateOp(converter, var);
+
+  if (sym.test(Fortran::semantics::Symbol::Flag::OmpDeclareTarget))
+    Fortran::lower::genDeclareTargetIntGlobal(converter, var);
+}
+
 int64_t Fortran::lower::getCollapseValue(
     const Fortran::parser::OmpClauseList &clauseList) {
   for (const Fortran::parser::OmpClause &clause : clauseList.v) {

>From ef5a70a1bd4d484f6079e0d55a80e943072ce218 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Fri, 15 Dec 2023 08:28:10 -0600
Subject: [PATCH 2/3] Remove braces around single statement

---
 flang/lib/Lower/Bridge.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 1ec242fafc5b6d..49700744db5181 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -4243,9 +4243,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
   void instantiateVar(const Fortran::lower::pft::Variable &var,
                       Fortran::lower::AggregateStoreMap &storeMap) {
     Fortran::lower::instantiateVariable(*this, var, localSymbols, storeMap);
-    if (var.hasSymbol()) {
+    if (var.hasSymbol())
       handleOpenMPSymbolProperties(*this, var);
-    }
   }
 
   /// Where applicable, save the exception state and halting and rounding

>From 906fc9df8aa8a6c559d47dba8573a04d39193054 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Fri, 15 Dec 2023 08:34:36 -0600
Subject: [PATCH 3/3] Rename handleOpenMP... to genOpenMP..., add inine
 documentation

---
 flang/include/flang/Lower/OpenMP.h | 9 +++++++--
 flang/lib/Lower/Bridge.cpp         | 2 +-
 flang/lib/Lower/OpenMP.cpp         | 2 +-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/flang/include/flang/Lower/OpenMP.h b/flang/include/flang/Lower/OpenMP.h
index a6ea26ee949fe7..a3b01a79e70331 100644
--- a/flang/include/flang/Lower/OpenMP.h
+++ b/flang/include/flang/Lower/OpenMP.h
@@ -56,8 +56,13 @@ void genOpenMPConstruct(AbstractConverter &, semantics::SemanticsContext &,
                         pft::Evaluation &, const parser::OpenMPConstruct &);
 void genOpenMPDeclarativeConstruct(AbstractConverter &, pft::Evaluation &,
                                    const parser::OpenMPDeclarativeConstruct &);
-void handleOpenMPSymbolProperties(AbstractConverter &converter,
-                                  const pft::Variable &var);
+/// Symbols in OpenMP code can have flags (e.g. threadprivate directive)
+/// that require additional handling when lowering the corresponding
+/// variable. Perform such handling according to the flags on the symbol.
+/// The variable \p var is required to have a `Symbol`.
+void genOpenMPSymbolProperties(AbstractConverter &converter,
+                               const pft::Variable &var);
+
 int64_t getCollapseValue(const Fortran::parser::OmpClauseList &clauseList);
 void genThreadprivateOp(AbstractConverter &, const pft::Variable &);
 void genDeclareTargetIntGlobal(AbstractConverter &, const pft::Variable &);
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 49700744db5181..586c9bbc2af96e 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -4244,7 +4244,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
                       Fortran::lower::AggregateStoreMap &storeMap) {
     Fortran::lower::instantiateVariable(*this, var, localSymbols, storeMap);
     if (var.hasSymbol())
-      handleOpenMPSymbolProperties(*this, var);
+      genOpenMPSymbolProperties(*this, var);
   }
 
   /// Where applicable, save the exception state and halting and rounding
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 9a798e641d1267..7acddd248745ad 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -3483,7 +3483,7 @@ void Fortran::lower::genOpenMPDeclarativeConstruct(
       ompDeclConstruct.u);
 }
 
-void Fortran::lower::handleOpenMPSymbolProperties(
+void Fortran::lower::genOpenMPSymbolProperties(
     Fortran::lower::AbstractConverter &converter,
     const Fortran::lower::pft::Variable &var) {
   assert(var.hasSymbol() && "Expecting Symbol");



More information about the flang-commits mailing list