[Flang][OpenMP] Update declare mapper lookup via use-module #163860
+219
−82
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Implemented semantic TODO to catch undeclared mappers. - Fix mapper lookup to include modules imported through USE. - Update and add tests. Fixes llvm#163385.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-parser @llvm/pr-subscribers-flang-fir-hlfir Author: Akash Banerjee (TIFitis) Changes
Fixes #163385. Full diff: https://github.com/llvm/llvm-project/pull/163860.diff 5 Files Affected:
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 85398be778382..41820c74e3921 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1402,10 +1402,14 @@ bool ClauseProcessor::processMap(
}
if (mappers) {
assert(mappers->size() == 1 && "more than one mapper");
- mapperIdName = mappers->front().v.id().symbol->name().ToString();
- if (mapperIdName != "default")
- mapperIdName = converter.mangleName(
- mapperIdName, mappers->front().v.id().symbol->owner());
+ const semantics::Symbol *mapperSym = mappers->front().v.id().symbol;
+ mapperIdName = mapperSym->name().ToString();
+ if (mapperIdName != "default") {
+ // Mangle with the ultimate owner so that use-associated mapper
+ // identifiers resolve to the same symbol as their defining scope.
+ const semantics::Symbol &ultimate = mapperSym->GetUltimate();
+ mapperIdName = converter.mangleName(mapperIdName, ultimate.owner());
+ }
}
processMapObjects(stmtCtx, clauseLocation,
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index ae0ff9ca8068d..ef45c692acc8c 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1824,21 +1824,24 @@ bool OmpVisitor::Pre(const parser::OmpMapClause &x) {
// TODO: Do we need a specific flag or type here, to distinghuish against
// other ConstructName things? Leaving this for the full implementation
// of mapper lowering.
- auto *misc{symbol->detailsIf<MiscDetails>()};
+ const Symbol &ultimate{symbol->GetUltimate()};
+ auto *misc{const_cast<Symbol &>(ultimate).detailsIf<MiscDetails>()};
if (!misc || misc->kind() != MiscDetails::Kind::ConstructName)
context().Say(mapper->v.source,
"Name '%s' should be a mapper name"_err_en_US, mapper->v.source);
else
mapper->v.symbol = symbol;
} else {
- mapper->v.symbol =
- &MakeSymbol(mapper->v, MiscDetails{MiscDetails::Kind::ConstructName});
- // TODO: When completing the implementation, we probably want to error if
- // the symbol is not declared, but right now, testing that the TODO for
- // OmpMapClause happens is obscured by the TODO for declare mapper, so
- // leaving this out. Remove the above line once the declare mapper is
- // implemented. context().Say(mapper->v.source, "'%s' not
- // declared"_err_en_US, mapper->v.source);
+ // Allow the special 'default' mapper identifier without prior
+ // declaration so lowering can recognize and handle it. Emit an
+ // error for any other missing mapper identifier.
+ if (mapper->v.source.ToString() == "default") {
+ mapper->v.symbol = &MakeSymbol(
+ mapper->v, MiscDetails{MiscDetails::Kind::ConstructName});
+ } else {
+ context().Say(
+ mapper->v.source, "'%s' not declared"_err_en_US, mapper->v.source);
+ }
}
}
return true;
@@ -3614,10 +3617,18 @@ void ModuleVisitor::Post(const parser::UseStmt &x) {
rename.u);
}
for (const auto &[name, symbol] : *useModuleScope_) {
+ // Default USE imports public names, excluding intrinsic-only and most
+ // miscellaneous details. However, allow OpenMP mapper identifiers,
+ // which are currently represented with MiscDetails::ConstructName.
+ bool isMapper{false};
+ if (const auto *misc{symbol->detailsIf<MiscDetails>()}) {
+ isMapper = misc->kind() == MiscDetails::Kind::ConstructName;
+ }
if (symbol->attrs().test(Attr::PUBLIC) && !IsUseRenamed(symbol->name()) &&
(!symbol->implicitAttrs().test(Attr::INTRINSIC) ||
symbol->has<UseDetails>()) &&
- !symbol->has<MiscDetails>() && useNames.count(name) == 0) {
+ (!symbol->has<MiscDetails>() || isMapper) &&
+ useNames.count(name) == 0) {
SourceName location{x.moduleName.source};
if (auto *localSymbol{FindInScope(name)}) {
DoAddUse(location, localSymbol->name(), *localSymbol, *symbol);
diff --git a/flang/test/Lower/OpenMP/declare-mapper.f90 b/flang/test/Lower/OpenMP/declare-mapper.f90
index 3d4d0da1e18a3..a5fb0710592ce 100644
--- a/flang/test/Lower/OpenMP/declare-mapper.f90
+++ b/flang/test/Lower/OpenMP/declare-mapper.f90
@@ -6,7 +6,8 @@
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-3.f90 -o - | FileCheck %t/omp-declare-mapper-3.f90
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-4.f90 -o - | FileCheck %t/omp-declare-mapper-4.f90
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-5.f90 -o - | FileCheck %t/omp-declare-mapper-5.f90
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=51 %t/omp-declare-mapper-6.f90 -o - | FileCheck %t/omp-declare-mapper-6.f90
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-6.f90 -o - | FileCheck %t/omp-declare-mapper-6.f90
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-7.f90 -o - | FileCheck %t/omp-declare-mapper-7.f90
!--- omp-declare-mapper-1.f90
subroutine declare_mapper_1
@@ -301,3 +302,25 @@ subroutine declare_mapper_nested_parent
r%real_arr = r%base_arr(1) + r%inner%deep_arr(1)
!$omp end target
end subroutine declare_mapper_nested_parent
+
+!--- omp-declare-mapper-7.f90
+! Check mappers declared inside modules and used via USE association.
+module m_mod
+ implicit none
+ type :: mty
+ integer :: x
+ end type mty
+ !$omp declare mapper(mymap : mty :: v) map(tofrom: v%x)
+end module m_mod
+
+program use_module_mapper
+ use m_mod
+ implicit none
+ type(mty) :: a
+
+ ! CHECK: omp.declare_mapper @[[MODMAP:[^ ]*mymap]]
+ ! CHECK: %{{.*}} = omp.map.info {{.*}} mapper(@[[MODMAP]]) {{.*}} {name = "a"}
+ !$omp target map(mapper(mymap) : a)
+ a%x = 42
+ !$omp end target
+end program use_module_mapper
diff --git a/flang/test/Parser/OpenMP/map-modifiers.f90 b/flang/test/Parser/OpenMP/map-modifiers.f90
index 83662b70f08f5..7d9b8856ac833 100644
--- a/flang/test/Parser/OpenMP/map-modifiers.f90
+++ b/flang/test/Parser/OpenMP/map-modifiers.f90
@@ -320,7 +320,7 @@ subroutine f21(x, y)
integer :: x(10)
integer :: y
integer, parameter :: p = 23
- !$omp target map(mapper(xx), from: x)
+ !$omp target map(mapper(default), from: x)
x = x + 1
!$omp end target
end
@@ -329,7 +329,7 @@ subroutine f21(x, y)
!UNPARSE: INTEGER x(10_4)
!UNPARSE: INTEGER y
!UNPARSE: INTEGER, PARAMETER :: p = 23_4
-!UNPARSE: !$OMP TARGET MAP(MAPPER(XX), FROM: X)
+!UNPARSE: !$OMP TARGET MAP(MAPPER(DEFAULT), FROM: X)
!UNPARSE: x=x+1_4
!UNPARSE: !$OMP END TARGET
!UNPARSE: END SUBROUTINE
@@ -337,7 +337,7 @@ subroutine f21(x, y)
!PARSE-TREE: OmpBeginDirective
!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = target
!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | Modifier -> OmpMapper -> Name = 'xx'
+!PARSE-TREE: | | Modifier -> OmpMapper -> Name = 'default'
!PARSE-TREE: | | Modifier -> OmpMapType -> Value = From
!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
@@ -375,4 +375,3 @@ subroutine f22(x)
!PARSE-TREE: | | SectionSubscript -> Integer -> Expr = 'i'
!PARSE-TREE: | | | Designator -> DataRef -> Name = 'i'
!PARSE-TREE: | bool = 'true'
-
diff --git a/flang/test/Semantics/OpenMP/map-clause-symbols.f90 b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
index 1d6315b4a2312..cdb65a71e8887 100644
--- a/flang/test/Semantics/OpenMP/map-clause-symbols.f90
+++ b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
@@ -1,6 +1,5 @@
-! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s
+! RUN: not %flang_fc1 -fopenmp -fopenmp-version=50 %s 2>&1 | FileCheck %s
program main
-!CHECK-LABEL: MainProgram scope: MAIN
integer, parameter :: n = 256
real(8) :: a(256)
!$omp target map(mapper(xx), from:a)
@@ -8,7 +7,6 @@ program main
a(i) = 4.2
end do
!$omp end target
-!CHECK: OtherConstruct scope: size=0 alignment=1 sourceRange=74 bytes
-!CHECK: OtherClause scope: size=0 alignment=1 sourceRange=0 bytes
-!CHECK: xx: Misc ConstructName
end program main
+
+! CHECK: error: '{{.*}}' not declared
|
|
@llvm/pr-subscribers-flang-openmp Author: Akash Banerjee (TIFitis) Changes
Fixes #163385. Full diff: https://github.com/llvm/llvm-project/pull/163860.diff 5 Files Affected:
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 85398be778382..41820c74e3921 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1402,10 +1402,14 @@ bool ClauseProcessor::processMap(
}
if (mappers) {
assert(mappers->size() == 1 && "more than one mapper");
- mapperIdName = mappers->front().v.id().symbol->name().ToString();
- if (mapperIdName != "default")
- mapperIdName = converter.mangleName(
- mapperIdName, mappers->front().v.id().symbol->owner());
+ const semantics::Symbol *mapperSym = mappers->front().v.id().symbol;
+ mapperIdName = mapperSym->name().ToString();
+ if (mapperIdName != "default") {
+ // Mangle with the ultimate owner so that use-associated mapper
+ // identifiers resolve to the same symbol as their defining scope.
+ const semantics::Symbol &ultimate = mapperSym->GetUltimate();
+ mapperIdName = converter.mangleName(mapperIdName, ultimate.owner());
+ }
}
processMapObjects(stmtCtx, clauseLocation,
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index ae0ff9ca8068d..ef45c692acc8c 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1824,21 +1824,24 @@ bool OmpVisitor::Pre(const parser::OmpMapClause &x) {
// TODO: Do we need a specific flag or type here, to distinghuish against
// other ConstructName things? Leaving this for the full implementation
// of mapper lowering.
- auto *misc{symbol->detailsIf<MiscDetails>()};
+ const Symbol &ultimate{symbol->GetUltimate()};
+ auto *misc{const_cast<Symbol &>(ultimate).detailsIf<MiscDetails>()};
if (!misc || misc->kind() != MiscDetails::Kind::ConstructName)
context().Say(mapper->v.source,
"Name '%s' should be a mapper name"_err_en_US, mapper->v.source);
else
mapper->v.symbol = symbol;
} else {
- mapper->v.symbol =
- &MakeSymbol(mapper->v, MiscDetails{MiscDetails::Kind::ConstructName});
- // TODO: When completing the implementation, we probably want to error if
- // the symbol is not declared, but right now, testing that the TODO for
- // OmpMapClause happens is obscured by the TODO for declare mapper, so
- // leaving this out. Remove the above line once the declare mapper is
- // implemented. context().Say(mapper->v.source, "'%s' not
- // declared"_err_en_US, mapper->v.source);
+ // Allow the special 'default' mapper identifier without prior
+ // declaration so lowering can recognize and handle it. Emit an
+ // error for any other missing mapper identifier.
+ if (mapper->v.source.ToString() == "default") {
+ mapper->v.symbol = &MakeSymbol(
+ mapper->v, MiscDetails{MiscDetails::Kind::ConstructName});
+ } else {
+ context().Say(
+ mapper->v.source, "'%s' not declared"_err_en_US, mapper->v.source);
+ }
}
}
return true;
@@ -3614,10 +3617,18 @@ void ModuleVisitor::Post(const parser::UseStmt &x) {
rename.u);
}
for (const auto &[name, symbol] : *useModuleScope_) {
+ // Default USE imports public names, excluding intrinsic-only and most
+ // miscellaneous details. However, allow OpenMP mapper identifiers,
+ // which are currently represented with MiscDetails::ConstructName.
+ bool isMapper{false};
+ if (const auto *misc{symbol->detailsIf<MiscDetails>()}) {
+ isMapper = misc->kind() == MiscDetails::Kind::ConstructName;
+ }
if (symbol->attrs().test(Attr::PUBLIC) && !IsUseRenamed(symbol->name()) &&
(!symbol->implicitAttrs().test(Attr::INTRINSIC) ||
symbol->has<UseDetails>()) &&
- !symbol->has<MiscDetails>() && useNames.count(name) == 0) {
+ (!symbol->has<MiscDetails>() || isMapper) &&
+ useNames.count(name) == 0) {
SourceName location{x.moduleName.source};
if (auto *localSymbol{FindInScope(name)}) {
DoAddUse(location, localSymbol->name(), *localSymbol, *symbol);
diff --git a/flang/test/Lower/OpenMP/declare-mapper.f90 b/flang/test/Lower/OpenMP/declare-mapper.f90
index 3d4d0da1e18a3..a5fb0710592ce 100644
--- a/flang/test/Lower/OpenMP/declare-mapper.f90
+++ b/flang/test/Lower/OpenMP/declare-mapper.f90
@@ -6,7 +6,8 @@
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-3.f90 -o - | FileCheck %t/omp-declare-mapper-3.f90
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-4.f90 -o - | FileCheck %t/omp-declare-mapper-4.f90
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-5.f90 -o - | FileCheck %t/omp-declare-mapper-5.f90
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=51 %t/omp-declare-mapper-6.f90 -o - | FileCheck %t/omp-declare-mapper-6.f90
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-6.f90 -o - | FileCheck %t/omp-declare-mapper-6.f90
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-7.f90 -o - | FileCheck %t/omp-declare-mapper-7.f90
!--- omp-declare-mapper-1.f90
subroutine declare_mapper_1
@@ -301,3 +302,25 @@ subroutine declare_mapper_nested_parent
r%real_arr = r%base_arr(1) + r%inner%deep_arr(1)
!$omp end target
end subroutine declare_mapper_nested_parent
+
+!--- omp-declare-mapper-7.f90
+! Check mappers declared inside modules and used via USE association.
+module m_mod
+ implicit none
+ type :: mty
+ integer :: x
+ end type mty
+ !$omp declare mapper(mymap : mty :: v) map(tofrom: v%x)
+end module m_mod
+
+program use_module_mapper
+ use m_mod
+ implicit none
+ type(mty) :: a
+
+ ! CHECK: omp.declare_mapper @[[MODMAP:[^ ]*mymap]]
+ ! CHECK: %{{.*}} = omp.map.info {{.*}} mapper(@[[MODMAP]]) {{.*}} {name = "a"}
+ !$omp target map(mapper(mymap) : a)
+ a%x = 42
+ !$omp end target
+end program use_module_mapper
diff --git a/flang/test/Parser/OpenMP/map-modifiers.f90 b/flang/test/Parser/OpenMP/map-modifiers.f90
index 83662b70f08f5..7d9b8856ac833 100644
--- a/flang/test/Parser/OpenMP/map-modifiers.f90
+++ b/flang/test/Parser/OpenMP/map-modifiers.f90
@@ -320,7 +320,7 @@ subroutine f21(x, y)
integer :: x(10)
integer :: y
integer, parameter :: p = 23
- !$omp target map(mapper(xx), from: x)
+ !$omp target map(mapper(default), from: x)
x = x + 1
!$omp end target
end
@@ -329,7 +329,7 @@ subroutine f21(x, y)
!UNPARSE: INTEGER x(10_4)
!UNPARSE: INTEGER y
!UNPARSE: INTEGER, PARAMETER :: p = 23_4
-!UNPARSE: !$OMP TARGET MAP(MAPPER(XX), FROM: X)
+!UNPARSE: !$OMP TARGET MAP(MAPPER(DEFAULT), FROM: X)
!UNPARSE: x=x+1_4
!UNPARSE: !$OMP END TARGET
!UNPARSE: END SUBROUTINE
@@ -337,7 +337,7 @@ subroutine f21(x, y)
!PARSE-TREE: OmpBeginDirective
!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = target
!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | Modifier -> OmpMapper -> Name = 'xx'
+!PARSE-TREE: | | Modifier -> OmpMapper -> Name = 'default'
!PARSE-TREE: | | Modifier -> OmpMapType -> Value = From
!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
@@ -375,4 +375,3 @@ subroutine f22(x)
!PARSE-TREE: | | SectionSubscript -> Integer -> Expr = 'i'
!PARSE-TREE: | | | Designator -> DataRef -> Name = 'i'
!PARSE-TREE: | bool = 'true'
-
diff --git a/flang/test/Semantics/OpenMP/map-clause-symbols.f90 b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
index 1d6315b4a2312..cdb65a71e8887 100644
--- a/flang/test/Semantics/OpenMP/map-clause-symbols.f90
+++ b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
@@ -1,6 +1,5 @@
-! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s
+! RUN: not %flang_fc1 -fopenmp -fopenmp-version=50 %s 2>&1 | FileCheck %s
program main
-!CHECK-LABEL: MainProgram scope: MAIN
integer, parameter :: n = 256
real(8) :: a(256)
!$omp target map(mapper(xx), from:a)
@@ -8,7 +7,6 @@ program main
a(i) = 4.2
end do
!$omp end target
-!CHECK: OtherConstruct scope: size=0 alignment=1 sourceRange=74 bytes
-!CHECK: OtherClause scope: size=0 alignment=1 sourceRange=0 bytes
-!CHECK: xx: Misc ConstructName
end program main
+
+! CHECK: error: '{{.*}}' not declared
|
|
@llvm/pr-subscribers-flang-semantics Author: Akash Banerjee (TIFitis) Changes
Fixes #163385. Full diff: https://github.com/llvm/llvm-project/pull/163860.diff 5 Files Affected:
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 85398be778382..41820c74e3921 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -1402,10 +1402,14 @@ bool ClauseProcessor::processMap(
}
if (mappers) {
assert(mappers->size() == 1 && "more than one mapper");
- mapperIdName = mappers->front().v.id().symbol->name().ToString();
- if (mapperIdName != "default")
- mapperIdName = converter.mangleName(
- mapperIdName, mappers->front().v.id().symbol->owner());
+ const semantics::Symbol *mapperSym = mappers->front().v.id().symbol;
+ mapperIdName = mapperSym->name().ToString();
+ if (mapperIdName != "default") {
+ // Mangle with the ultimate owner so that use-associated mapper
+ // identifiers resolve to the same symbol as their defining scope.
+ const semantics::Symbol &ultimate = mapperSym->GetUltimate();
+ mapperIdName = converter.mangleName(mapperIdName, ultimate.owner());
+ }
}
processMapObjects(stmtCtx, clauseLocation,
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index ae0ff9ca8068d..ef45c692acc8c 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1824,21 +1824,24 @@ bool OmpVisitor::Pre(const parser::OmpMapClause &x) {
// TODO: Do we need a specific flag or type here, to distinghuish against
// other ConstructName things? Leaving this for the full implementation
// of mapper lowering.
- auto *misc{symbol->detailsIf<MiscDetails>()};
+ const Symbol &ultimate{symbol->GetUltimate()};
+ auto *misc{const_cast<Symbol &>(ultimate).detailsIf<MiscDetails>()};
if (!misc || misc->kind() != MiscDetails::Kind::ConstructName)
context().Say(mapper->v.source,
"Name '%s' should be a mapper name"_err_en_US, mapper->v.source);
else
mapper->v.symbol = symbol;
} else {
- mapper->v.symbol =
- &MakeSymbol(mapper->v, MiscDetails{MiscDetails::Kind::ConstructName});
- // TODO: When completing the implementation, we probably want to error if
- // the symbol is not declared, but right now, testing that the TODO for
- // OmpMapClause happens is obscured by the TODO for declare mapper, so
- // leaving this out. Remove the above line once the declare mapper is
- // implemented. context().Say(mapper->v.source, "'%s' not
- // declared"_err_en_US, mapper->v.source);
+ // Allow the special 'default' mapper identifier without prior
+ // declaration so lowering can recognize and handle it. Emit an
+ // error for any other missing mapper identifier.
+ if (mapper->v.source.ToString() == "default") {
+ mapper->v.symbol = &MakeSymbol(
+ mapper->v, MiscDetails{MiscDetails::Kind::ConstructName});
+ } else {
+ context().Say(
+ mapper->v.source, "'%s' not declared"_err_en_US, mapper->v.source);
+ }
}
}
return true;
@@ -3614,10 +3617,18 @@ void ModuleVisitor::Post(const parser::UseStmt &x) {
rename.u);
}
for (const auto &[name, symbol] : *useModuleScope_) {
+ // Default USE imports public names, excluding intrinsic-only and most
+ // miscellaneous details. However, allow OpenMP mapper identifiers,
+ // which are currently represented with MiscDetails::ConstructName.
+ bool isMapper{false};
+ if (const auto *misc{symbol->detailsIf<MiscDetails>()}) {
+ isMapper = misc->kind() == MiscDetails::Kind::ConstructName;
+ }
if (symbol->attrs().test(Attr::PUBLIC) && !IsUseRenamed(symbol->name()) &&
(!symbol->implicitAttrs().test(Attr::INTRINSIC) ||
symbol->has<UseDetails>()) &&
- !symbol->has<MiscDetails>() && useNames.count(name) == 0) {
+ (!symbol->has<MiscDetails>() || isMapper) &&
+ useNames.count(name) == 0) {
SourceName location{x.moduleName.source};
if (auto *localSymbol{FindInScope(name)}) {
DoAddUse(location, localSymbol->name(), *localSymbol, *symbol);
diff --git a/flang/test/Lower/OpenMP/declare-mapper.f90 b/flang/test/Lower/OpenMP/declare-mapper.f90
index 3d4d0da1e18a3..a5fb0710592ce 100644
--- a/flang/test/Lower/OpenMP/declare-mapper.f90
+++ b/flang/test/Lower/OpenMP/declare-mapper.f90
@@ -6,7 +6,8 @@
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-3.f90 -o - | FileCheck %t/omp-declare-mapper-3.f90
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-4.f90 -o - | FileCheck %t/omp-declare-mapper-4.f90
! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-5.f90 -o - | FileCheck %t/omp-declare-mapper-5.f90
-! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=51 %t/omp-declare-mapper-6.f90 -o - | FileCheck %t/omp-declare-mapper-6.f90
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-6.f90 -o - | FileCheck %t/omp-declare-mapper-6.f90
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 %t/omp-declare-mapper-7.f90 -o - | FileCheck %t/omp-declare-mapper-7.f90
!--- omp-declare-mapper-1.f90
subroutine declare_mapper_1
@@ -301,3 +302,25 @@ subroutine declare_mapper_nested_parent
r%real_arr = r%base_arr(1) + r%inner%deep_arr(1)
!$omp end target
end subroutine declare_mapper_nested_parent
+
+!--- omp-declare-mapper-7.f90
+! Check mappers declared inside modules and used via USE association.
+module m_mod
+ implicit none
+ type :: mty
+ integer :: x
+ end type mty
+ !$omp declare mapper(mymap : mty :: v) map(tofrom: v%x)
+end module m_mod
+
+program use_module_mapper
+ use m_mod
+ implicit none
+ type(mty) :: a
+
+ ! CHECK: omp.declare_mapper @[[MODMAP:[^ ]*mymap]]
+ ! CHECK: %{{.*}} = omp.map.info {{.*}} mapper(@[[MODMAP]]) {{.*}} {name = "a"}
+ !$omp target map(mapper(mymap) : a)
+ a%x = 42
+ !$omp end target
+end program use_module_mapper
diff --git a/flang/test/Parser/OpenMP/map-modifiers.f90 b/flang/test/Parser/OpenMP/map-modifiers.f90
index 83662b70f08f5..7d9b8856ac833 100644
--- a/flang/test/Parser/OpenMP/map-modifiers.f90
+++ b/flang/test/Parser/OpenMP/map-modifiers.f90
@@ -320,7 +320,7 @@ subroutine f21(x, y)
integer :: x(10)
integer :: y
integer, parameter :: p = 23
- !$omp target map(mapper(xx), from: x)
+ !$omp target map(mapper(default), from: x)
x = x + 1
!$omp end target
end
@@ -329,7 +329,7 @@ subroutine f21(x, y)
!UNPARSE: INTEGER x(10_4)
!UNPARSE: INTEGER y
!UNPARSE: INTEGER, PARAMETER :: p = 23_4
-!UNPARSE: !$OMP TARGET MAP(MAPPER(XX), FROM: X)
+!UNPARSE: !$OMP TARGET MAP(MAPPER(DEFAULT), FROM: X)
!UNPARSE: x=x+1_4
!UNPARSE: !$OMP END TARGET
!UNPARSE: END SUBROUTINE
@@ -337,7 +337,7 @@ subroutine f21(x, y)
!PARSE-TREE: OmpBeginDirective
!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = target
!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause
-!PARSE-TREE: | | Modifier -> OmpMapper -> Name = 'xx'
+!PARSE-TREE: | | Modifier -> OmpMapper -> Name = 'default'
!PARSE-TREE: | | Modifier -> OmpMapType -> Value = From
!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
@@ -375,4 +375,3 @@ subroutine f22(x)
!PARSE-TREE: | | SectionSubscript -> Integer -> Expr = 'i'
!PARSE-TREE: | | | Designator -> DataRef -> Name = 'i'
!PARSE-TREE: | bool = 'true'
-
diff --git a/flang/test/Semantics/OpenMP/map-clause-symbols.f90 b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
index 1d6315b4a2312..cdb65a71e8887 100644
--- a/flang/test/Semantics/OpenMP/map-clause-symbols.f90
+++ b/flang/test/Semantics/OpenMP/map-clause-symbols.f90
@@ -1,6 +1,5 @@
-! RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=50 %s | FileCheck %s
+! RUN: not %flang_fc1 -fopenmp -fopenmp-version=50 %s 2>&1 | FileCheck %s
program main
-!CHECK-LABEL: MainProgram scope: MAIN
integer, parameter :: n = 256
real(8) :: a(256)
!$omp target map(mapper(xx), from:a)
@@ -8,7 +7,6 @@ program main
a(i) = 4.2
end do
!$omp end target
-!CHECK: OtherConstruct scope: size=0 alignment=1 sourceRange=74 bytes
-!CHECK: OtherClause scope: size=0 alignment=1 sourceRange=0 bytes
-!CHECK: xx: Misc ConstructName
end program main
+
+! CHECK: error: '{{.*}}' not declared
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the handling of OpenMP declare mapper identifiers when they are imported via USE statements from modules. The changes address issue #163385 by implementing proper semantic checking for undeclared mappers and ensuring mapper lookup includes module-imported identifiers.
Key Changes
- Implemented semantic error checking to catch undeclared mapper identifiers (except for the special "default" mapper)
- Fixed mapper symbol resolution to use the ultimate symbol owner for proper mangling when mappers are imported via USE
- Added support for importing OpenMP mapper identifiers through module USE statements
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| flang/test/Semantics/OpenMP/map-clause-symbols.f90 | Updated test to verify error reporting for undeclared mapper identifiers |
| flang/test/Parser/OpenMP/map-modifiers.f90 | Changed test to use "default" mapper instead of undeclared "xx" mapper |
| flang/test/Lower/OpenMP/declare-mapper.f90 | Added new test case for mapper usage via module USE association |
| flang/lib/Semantics/resolve-names.cpp | Implemented error checking for undeclared mappers and enabled mapper import through USE statements |
| flang/lib/Lower/OpenMP/ClauseProcessor.cpp | Fixed mapper name mangling to use ultimate symbol owner for use-associated mappers |
|
I don't see changes to add support (in lib/Semantics/mod-file.cpp) for emitting the |
I think it gets added here - llvm-project/flang/lib/Lower/OpenMP/OpenMP.cpp Line 3583 in 48a99ad |
|
Ping for review. |
I meant the module file (m_mod.mod) should have an entry for the declare mapper. Without this how will the mapper in Does the FIR/OMP code generation generate a dummy declare mapper if none is found? If so that is not complete. |
…e and program file compilation.
|
@kiranchandramohan Sorry for the earlier misunderstanding — I’m not very familiar with the parsing/semantic side of things. It was working without exporting to the .mod file because the test originally had both the module and the program in the same file. Once separated, it started failing. I’ve now updated the PR to export the declare mapper to the .mod file as you suggested, and the lookup works correctly when compiling across different files. I’ve also updated the test accordingly. As far as I’m aware, FIR/OMP does not emit dummy mappers at any stage. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #163385.