[llvm-branch-commits] [flang] [flang][OpenMP] Use OmpDirectiveSpecification in DECLARE_MAPPER (PR #160169)

Krzysztof Parzyszek via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon Sep 22 12:50:03 PDT 2025


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

>From db95ffd7044e5155c7a5ec2a0022a928ee261a55 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Fri, 19 Sep 2025 08:52:08 -0500
Subject: [PATCH 1/3] [flang][OpenMP] Use OmpDirectiveSpecification in
 DECLARE_MAPPER

---
 flang/include/flang/Parser/openmp-utils.h     |  2 --
 flang/include/flang/Parser/parse-tree.h       |  4 +--
 flang/lib/Lower/OpenMP/OpenMP.cpp             | 20 ++++++------
 flang/lib/Parser/openmp-parsers.cpp           |  5 +--
 flang/lib/Parser/unparse.cpp                  | 17 ++--------
 flang/lib/Semantics/check-omp-structure.cpp   | 31 ++++++++++++-------
 flang/lib/Semantics/resolve-directives.cpp    |  3 +-
 flang/lib/Semantics/resolve-names.cpp         |  5 ++-
 .../Parser/OpenMP/declare-mapper-unparse.f90  |  4 +--
 .../OpenMP/openmp6-directive-spellings.f90    |  9 +++---
 .../Semantics/OpenMP/declare-mapper04.f90     | 18 +++++++++++
 11 files changed, 66 insertions(+), 52 deletions(-)
 create mode 100644 flang/test/Semantics/OpenMP/declare-mapper04.f90

diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index 98e849eef9bbc..19e23e4d9f62b 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -43,7 +43,6 @@ MAKE_CONSTR_ID(OmpErrorDirective, D::OMPD_error);
 MAKE_CONSTR_ID(OmpMetadirectiveDirective, D::OMPD_metadirective);
 MAKE_CONSTR_ID(OpenMPDeclarativeAllocate, D::OMPD_allocate);
 MAKE_CONSTR_ID(OpenMPDeclarativeAssumes, D::OMPD_assumes);
-MAKE_CONSTR_ID(OpenMPDeclareMapperConstruct, D::OMPD_declare_mapper);
 MAKE_CONSTR_ID(OpenMPDeclareReductionConstruct, D::OMPD_declare_reduction);
 MAKE_CONSTR_ID(OpenMPDeclareSimdConstruct, D::OMPD_declare_simd);
 MAKE_CONSTR_ID(OpenMPDeclareTargetConstruct, D::OMPD_declare_target);
@@ -105,7 +104,6 @@ struct DirectiveNameScope {
           std::is_same_v<T, OmpMetadirectiveDirective> ||
           std::is_same_v<T, OpenMPDeclarativeAllocate> ||
           std::is_same_v<T, OpenMPDeclarativeAssumes> ||
-          std::is_same_v<T, OpenMPDeclareMapperConstruct> ||
           std::is_same_v<T, OpenMPDeclareReductionConstruct> ||
           std::is_same_v<T, OpenMPDeclareSimdConstruct> ||
           std::is_same_v<T, OpenMPDeclareTargetConstruct> ||
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index bd0debe297916..73f8fbdbbb467 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4951,9 +4951,9 @@ struct OpenMPDeclareTargetConstruct {
 // OMP v5.2: 5.8.8
 //  declare-mapper -> DECLARE MAPPER ([mapper-name :] type :: var) map-clauses
 struct OpenMPDeclareMapperConstruct {
-  TUPLE_CLASS_BOILERPLATE(OpenMPDeclareMapperConstruct);
+  WRAPPER_CLASS_BOILERPLATE(
+      OpenMPDeclareMapperConstruct, OmpDirectiveSpecification);
   CharBlock source;
-  std::tuple<Verbatim, OmpMapperSpecifier, OmpClauseList> t;
 };
 
 // ref: 5.2: Section 5.5.11 139-141
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 5681be664d450..c549485565bad 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3444,15 +3444,17 @@ genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
 static void
 genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
        semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
-       const parser::OpenMPDeclareMapperConstruct &declareMapperConstruct) {
-  mlir::Location loc = converter.genLocation(declareMapperConstruct.source);
+       const parser::OpenMPDeclareMapperConstruct &construct) {
+  mlir::Location loc = converter.genLocation(construct.source);
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+  const parser::OmpArgumentList &args = construct.v.Arguments();
+  assert(args.v.size() == 1 && "Expecting single argument");
   lower::StatementContext stmtCtx;
-  const auto &spec =
-      std::get<parser::OmpMapperSpecifier>(declareMapperConstruct.t);
-  const auto &mapperName{std::get<std::string>(spec.t)};
-  const auto &varType{std::get<parser::TypeSpec>(spec.t)};
-  const auto &varName{std::get<parser::Name>(spec.t)};
+  const auto *spec = std::get_if<parser::OmpMapperSpecifier>(&args.v.front().u);
+  assert(spec && "Expecting mapper specifier");
+  const auto &mapperName{std::get<std::string>(spec->t)};
+  const auto &varType{std::get<parser::TypeSpec>(spec->t)};
+  const auto &varName{std::get<parser::Name>(spec->t)};
   assert(varType.declTypeSpec->category() ==
              semantics::DeclTypeSpec::Category::TypeDerived &&
          "Expected derived type");
@@ -3476,9 +3478,7 @@ genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
 
   // Populate the declareMapper region with the map information.
   mlir::omp::DeclareMapperInfoOperands clauseOps;
-  const auto *clauseList{
-      parser::Unwrap<parser::OmpClauseList>(declareMapperConstruct.t)};
-  List<Clause> clauses = makeClauses(*clauseList, semaCtx);
+  List<Clause> clauses = makeClauses(construct.v.Clauses(), semaCtx);
   ClauseProcessor cp(converter, semaCtx, clauses);
   cp.processMap(loc, stmtCtx, clauseOps);
   mlir::omp::DeclareMapperInfoOp::create(firOpBuilder, loc, clauseOps.mapVars);
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 8ab9905123135..dc638ca9d00fe 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1756,8 +1756,9 @@ TYPE_PARSER(applyFunction<OmpMapperSpecifier>(ConstructOmpMapperSpecifier,
 
 // OpenMP 5.2: 5.8.8 Declare Mapper Construct
 TYPE_PARSER(sourced(construct<OpenMPDeclareMapperConstruct>(
-    verbatim("DECLARE MAPPER"_tok) || verbatim("DECLARE_MAPPER"_tok),
-    parenthesized(Parser<OmpMapperSpecifier>{}), Parser<OmpClauseList>{})))
+    predicated(Parser<OmpDirectiveName>{},
+        IsDirective(llvm::omp::Directive::OMPD_declare_mapper)) >=
+    Parser<OmpDirectiveSpecification>{})))
 
 TYPE_PARSER(construct<OmpReductionCombiner>(Parser<AssignmentStmt>{}) ||
     construct<OmpReductionCombiner>(Parser<FunctionReference>{}))
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index b06a8c1fa4374..01b0e32a1164e 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2547,21 +2547,10 @@ class UnparseVisitor {
     Put("\n");
     EndOpenMP();
   }
-  void Unparse(const OpenMPDeclareMapperConstruct &z) {
+  void Unparse(const OpenMPDeclareMapperConstruct &x) {
     BeginOpenMP();
-    Word("!$OMP DECLARE MAPPER (");
-    const auto &spec{std::get<OmpMapperSpecifier>(z.t)};
-    const auto &mapperName{std::get<std::string>(spec.t)};
-    if (mapperName.find(llvm::omp::OmpDefaultMapperName) == std::string::npos) {
-      Walk(mapperName);
-      Put(":");
-    }
-    Walk(std::get<TypeSpec>(spec.t));
-    Put("::");
-    Walk(std::get<Name>(spec.t));
-    Put(")");
-
-    Walk(std::get<OmpClauseList>(z.t));
+    Word("!$OMP ");
+    Walk(x.v);
     Put("\n");
     EndOpenMP();
   }
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index c39daef6b0ea9..17954b5826586 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -629,11 +629,6 @@ template <typename Checker> struct DirectiveSpellingVisitor {
     checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_assumes);
     return false;
   }
-  bool Pre(const parser::OpenMPDeclareMapperConstruct &x) {
-    checker_(
-        std::get<parser::Verbatim>(x.t).source, Directive::OMPD_declare_mapper);
-    return false;
-  }
   bool Pre(const parser::OpenMPDeclareReductionConstruct &x) {
     checker_(std::get<parser::Verbatim>(x.t).source,
         Directive::OMPD_declare_reduction);
@@ -1595,13 +1590,25 @@ void OmpStructureChecker::Leave(const parser::OmpDeclareTargetWithClause &x) {
 }
 
 void OmpStructureChecker::Enter(const parser::OpenMPDeclareMapperConstruct &x) {
-  const auto &dir{std::get<parser::Verbatim>(x.t)};
-  PushContextAndClauseSets(
-      dir.source, llvm::omp::Directive::OMPD_declare_mapper);
-  const auto &spec{std::get<parser::OmpMapperSpecifier>(x.t)};
-  const auto &type = std::get<parser::TypeSpec>(spec.t);
-  if (!std::get_if<parser::DerivedTypeSpec>(&type.u)) {
-    context_.Say(dir.source, "Type is not a derived type"_err_en_US);
+  const parser::OmpDirectiveName &dirName{x.v.DirName()};
+  PushContextAndClauseSets(dirName.source, dirName.v);
+
+  const parser::OmpArgumentList &args{x.v.Arguments()};
+  if (args.v.size() != 1) {
+    context_.Say(args.source,
+        "DECLARE_MAPPER directive should have a single argument"_err_en_US);
+    return;
+  }
+
+  const parser::OmpArgument &arg{args.v.front()};
+  if (auto *spec{std::get_if<parser::OmpMapperSpecifier>(&arg.u)}) {
+    const auto &type = std::get<parser::TypeSpec>(spec->t);
+    if (!std::get_if<parser::DerivedTypeSpec>(&type.u)) {
+      context_.Say(arg.source, "Type is not a derived type"_err_en_US);
+    }
+  } else {
+    context_.Say(arg.source,
+        "The argument to the DECLARE_MAPPER directive should be a mapper-specifier"_err_en_US);
   }
 }
 
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 2d1bec9968593..48cd0e54a155a 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2331,7 +2331,8 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareTargetConstruct &x) {
 }
 
 bool OmpAttributeVisitor::Pre(const parser::OpenMPDeclareMapperConstruct &x) {
-  PushContext(x.source, llvm::omp::Directive::OMPD_declare_mapper);
+  const parser::OmpDirectiveName &dirName{x.v.DirName()};
+  PushContext(dirName.source, dirName.v);
   return true;
 }
 
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 358bec20f243c..b6d2ba7c4344d 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1518,9 +1518,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
 
   bool Pre(const parser::OpenMPDeclareMapperConstruct &x) {
     AddOmpSourceRange(x.source);
-    ProcessMapperSpecifier(std::get<parser::OmpMapperSpecifier>(x.t),
-        std::get<parser::OmpClauseList>(x.t));
-    return false;
+    return true;
   }
 
   bool Pre(const parser::OpenMPDeclareSimdConstruct &x) {
@@ -1686,6 +1684,7 @@ class OmpVisitor : public virtual DeclarationVisitor {
       PopScope();
     }
   }
+  bool Pre(const parser::OmpMapperSpecifier &x) { return false; }
   bool Pre(const parser::OmpDirectiveSpecification &x);
   void Post(const parser::OmpDirectiveSpecification &) {
     messageHandler().set_currStmtSource(std::nullopt);
diff --git a/flang/test/Parser/OpenMP/declare-mapper-unparse.f90 b/flang/test/Parser/OpenMP/declare-mapper-unparse.f90
index 30d75d02736f3..b53bf5ce10557 100644
--- a/flang/test/Parser/OpenMP/declare-mapper-unparse.f90
+++ b/flang/test/Parser/OpenMP/declare-mapper-unparse.f90
@@ -9,7 +9,7 @@ program main
   end type ty
 
 
-!CHECK: !$OMP DECLARE MAPPER (mymapper:ty::mapped) MAP(mapped,mapped%x)
+!CHECK: !$OMP DECLARE MAPPER(mymapper:ty::mapped) MAP(mapped,mapped%x)
   !$omp declare mapper(mymapper : ty :: mapped) map(mapped, mapped%x)
 
 !PARSE-TREE:      OpenMPDeclareMapperConstruct
@@ -24,7 +24,7 @@ program main
 !PARSE-TREE:           DataRef -> Name = 'mapped'
 !PARSE-TREE:           Name = 'x'
 
-!CHECK: !$OMP DECLARE MAPPER (ty::mapped) MAP(mapped,mapped%x)
+!CHECK: !$OMP DECLARE MAPPER(ty::mapped) MAP(mapped,mapped%x)
   !$omp declare mapper(ty :: mapped) map(mapped, mapped%x)
 
 !PARSE-TREE:      OpenMPDeclareMapperConstruct
diff --git a/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90 b/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90
index c2498c878f559..47237de2d5aff 100644
--- a/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90
+++ b/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90
@@ -51,12 +51,12 @@ subroutine f01
 !UNPARSE:  TYPE :: t
 !UNPARSE:   INTEGER :: x
 !UNPARSE:  END TYPE
-!UNPARSE: !$OMP DECLARE MAPPER (t::v) MAP(v%x)
+!UNPARSE: !$OMP DECLARE_MAPPER(t::v) MAP(v%x)
 !UNPARSE: END SUBROUTINE
 
-!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareMapperConstruct
-!PARSE-TREE: | Verbatim
-!PARSE-TREE: | OmpMapperSpecifier
+!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareMapperConstruct -> OmpDirectiveSpecification
+!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = declare mapper
+!PARSE-TREE: | OmpArgumentList -> OmpArgument -> OmpMapperSpecifier
 !PARSE-TREE: | | string = 't.omp.default.mapper'
 !PARSE-TREE: | | TypeSpec -> DerivedTypeSpec
 !PARSE-TREE: | | | Name = 't'
@@ -66,6 +66,7 @@ subroutine f01
 !PARSE-TREE: | | | DataRef -> Name = 'v'
 !PARSE-TREE: | | | Name = 'x'
 !PARSE-TREE: | | bool = 'true'
+!PARSE-TREE: | Flags = None
 
 subroutine f02
   type :: t
diff --git a/flang/test/Semantics/OpenMP/declare-mapper04.f90 b/flang/test/Semantics/OpenMP/declare-mapper04.f90
new file mode 100644
index 0000000000000..2f45e230c3513
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/declare-mapper04.f90
@@ -0,0 +1,18 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=60
+
+type :: t1
+   integer :: y
+end type
+
+type :: t2
+   integer :: y
+end type
+
+!ERROR: DECLARE_MAPPER directive should have a single argument
+!$omp declare mapper(m1:t1::x, m2:t2::x) map(x, x%y)
+
+integer :: x(10)
+!ERROR: The argument to the DECLARE_MAPPER directive should be a mapper-specifier
+!$omp declare mapper(x) map(to: x)
+
+end

>From 7e3d8a74746878c864ba7821089a6dd28b053824 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 22 Sep 2025 13:36:17 -0500
Subject: [PATCH 2/3] format

---
 flang/lib/Lower/OpenMP/OpenMP.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index c549485565bad..d2e865b3e1d0c 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3441,10 +3441,10 @@ genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
     TODO(converter.getCurrentLocation(), "OpenMPDeclareSimdConstruct");
 }
 
-static void
-genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
-       semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
-       const parser::OpenMPDeclareMapperConstruct &construct) {
+static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
+                   semantics::SemanticsContext &semaCtx,
+                   lower::pft::Evaluation &eval,
+                   const parser::OpenMPDeclareMapperConstruct &construct) {
   mlir::Location loc = converter.genLocation(construct.source);
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   const parser::OmpArgumentList &args = construct.v.Arguments();

>From 3947c8326f562db035cbcb964d533c9077e552f9 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 22 Sep 2025 14:49:15 -0500
Subject: [PATCH 3/3] Abort when walk reaches OmpMapperSpecifier

If it happens, it's a bug to be fixed.
---
 flang/lib/Semantics/resolve-names.cpp | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index b6d2ba7c4344d..69ca86ec65242 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1684,7 +1684,11 @@ class OmpVisitor : public virtual DeclarationVisitor {
       PopScope();
     }
   }
-  bool Pre(const parser::OmpMapperSpecifier &x) { return false; }
+  bool Pre(const parser::OmpMapperSpecifier &x) {
+    // OmpMapperSpecifier is handled explicitly, and the Walk infrastructure
+    // should not reach the point where it calls this function.
+    llvm_unreachable("This function should not be reached by 'Walk'");
+  }
   bool Pre(const parser::OmpDirectiveSpecification &x);
   void Post(const parser::OmpDirectiveSpecification &) {
     messageHandler().set_currStmtSource(std::nullopt);



More information about the llvm-branch-commits mailing list