-
Notifications
You must be signed in to change notification settings - Fork 15k
[Flang][OpenMP] Update declare mapper lookup via use-module #163860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
80194f3
a8f1613
0298b81
a941777
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3553,10 +3553,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 genOpenMPDeclareMapperImpl( | ||||||||||||||||||
| lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx, | ||||||||||||||||||
| const parser::OpenMPDeclareMapperConstruct &construct, | ||||||||||||||||||
| const semantics::Symbol *mapperSymOpt = nullptr) { | ||||||||||||||||||
| mlir::Location loc = converter.genLocation(construct.source); | ||||||||||||||||||
| fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); | ||||||||||||||||||
| const parser::OmpArgumentList &args = construct.v.Arguments(); | ||||||||||||||||||
|
|
@@ -3572,8 +3572,17 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, | |||||||||||||||||
| "Expected derived type"); | ||||||||||||||||||
|
|
||||||||||||||||||
| std::string mapperNameStr = mapperName; | ||||||||||||||||||
| if (auto *sym = converter.getCurrentScope().FindSymbol(mapperNameStr)) | ||||||||||||||||||
| if (mapperSymOpt && mapperNameStr != "default") { | ||||||||||||||||||
| mapperNameStr = converter.mangleName(mapperNameStr, mapperSymOpt->owner()); | ||||||||||||||||||
| } else if (auto *sym = | ||||||||||||||||||
| converter.getCurrentScope().FindSymbol(mapperNameStr)) { | ||||||||||||||||||
| mapperNameStr = converter.mangleName(mapperNameStr, sym->owner()); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // If the mapper op already exists (e.g., created by regular lowering), | ||||||||||||||||||
| // do not recreate it. | ||||||||||||||||||
|
Comment on lines
+3582
to
+3583
|
||||||||||||||||||
| // If the mapper op already exists (e.g., created by regular lowering), | |
| // do not recreate it. | |
| // If the mapper op already exists (e.g., created by regular lowering or by | |
| // materialization of imported mappers), do not recreate it. |
Copilot
AI
Oct 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nested conditions can be simplified using early returns. Consider restructuring as: if (!root.symbol() || !root.symbol()->test(semantics::Symbol::Flag::ModFile)) return; to reduce nesting and improve readability.
| if (const semantics::Symbol *modSym = root.symbol()) { | |
| if (!modSym->test(semantics::Symbol::Flag::ModFile)) | |
| return; | |
| } else { | |
| return; | |
| } | |
| if (!root.symbol() || !root.symbol()->test(semantics::Symbol::Flag::ModFile)) | |
| return; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,7 @@ static void PutBound(llvm::raw_ostream &, const Bound &); | |
| static void PutShapeSpec(llvm::raw_ostream &, const ShapeSpec &); | ||
| static void PutShape( | ||
| llvm::raw_ostream &, const ArraySpec &, char open, char close); | ||
| static void PutMapper(llvm::raw_ostream &, const Symbol &, SemanticsContext &); | ||
|
|
||
| static llvm::raw_ostream &PutAttr(llvm::raw_ostream &, Attr); | ||
| static llvm::raw_ostream &PutType(llvm::raw_ostream &, const DeclTypeSpec &); | ||
|
|
@@ -938,6 +939,7 @@ void ModFileWriter::PutEntity(llvm::raw_ostream &os, const Symbol &symbol) { | |
| [&](const ProcEntityDetails &) { PutProcEntity(os, symbol); }, | ||
| [&](const TypeParamDetails &) { PutTypeParam(os, symbol); }, | ||
| [&](const UserReductionDetails &) { PutUserReduction(os, symbol); }, | ||
| [&](const MapperDetails &) { PutMapper(decls_, symbol, context_); }, | ||
| [&](const auto &) { | ||
| common::die("PutEntity: unexpected details: %s", | ||
| DetailsToString(symbol.details()).c_str()); | ||
|
|
@@ -1098,6 +1100,16 @@ void ModFileWriter::PutUserReduction( | |
| } | ||
| } | ||
|
|
||
| static void PutMapper( | ||
| llvm::raw_ostream &os, const Symbol &symbol, SemanticsContext &context) { | ||
| const auto &details{symbol.get<MapperDetails>()}; | ||
| // Emit each saved DECLARE MAPPER construct as-is, so that consumers of the | ||
| // module can reparse it and recreate the mapper symbol and semantics state. | ||
| for (const auto *decl : details.GetDeclList()) { | ||
| Unparse(os, *decl, context.langOptions()); | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
+1103
to
+1112
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a test that checks the presence of the declare mapper in the module file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| void PutInit(llvm::raw_ostream &os, const Symbol &symbol, const MaybeExpr &init, | ||
| const parser::Expr *unanalyzed, SemanticsContext &context) { | ||
| if (IsNamedConstant(symbol) || symbol.owner().IsDerivedType()) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| ! RUN: split-file %s %t | ||
| ! RUN: %flang_fc1 -fsyntax-only -fopenmp -fopenmp-version=50 -module-dir %t %t/m.f90 | ||
| ! RUN: cat %t/m.mod | FileCheck --ignore-case %s | ||
|
|
||
| !--- m.f90 | ||
| module m | ||
| implicit none | ||
| type :: t | ||
| integer :: x | ||
| end type t | ||
| !$omp declare mapper(mymap : t :: v) map(v%x) | ||
| end module m | ||
|
|
||
| !CHECK: !$OMP DECLARE MAPPER(mymap:t::v) MAP(v%x) |
Uh oh!
There was an error while loading. Please reload this page.