From 99b864d7b3a0621529459e8e27d765be475671e4 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 2 Apr 2025 10:52:21 -0500 Subject: [PATCH 1/5] [flang][OpenMP] Use function symbol on DECLARE TARGET Consider: ``` function foo() !$omp declare target(foo) ! This `foo` was a function-return symbol ... end ``` When resolving symbols, for this case use the symbol corresponding to the function instead of the symbol corresponding to the function result. --- flang/lib/Semantics/resolve-directives.cpp | 10 ++++++ flang/lib/Semantics/unparse-with-symbols.cpp | 8 +++++ .../OpenMP/declare-target-func-and-subr.f90 | 7 ++++ ...lare-target-function-name-with-symbols.f90 | 34 +++++++++++++++++++ 4 files changed, 59 insertions(+) create mode 100644 flang/test/Semantics/OpenMP/declare-target-function-name-with-symbols.f90 diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index f905da0a7239d..4ce43e32f4447 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -2514,6 +2514,16 @@ void OmpAttributeVisitor::ResolveOmpObject( name->ToString()); } } + if (ompFlag == Symbol::Flag::OmpDeclareTarget) { + if (symbol->IsFuncResult()) { + if (Scope *container{&currScope()}) { + if (Symbol *func{container->symbol()}) { + func->set(ompFlag); + name->symbol = func; + } + } + } + } if (GetContext().directive == llvm::omp::Directive::OMPD_target_data) { checkExclusivelists(symbol, Symbol::Flag::OmpUseDevicePtr, diff --git a/flang/lib/Semantics/unparse-with-symbols.cpp b/flang/lib/Semantics/unparse-with-symbols.cpp index 02afb89ae57fa..2716d88efb9fb 100644 --- a/flang/lib/Semantics/unparse-with-symbols.cpp +++ b/flang/lib/Semantics/unparse-with-symbols.cpp @@ -61,6 +61,14 @@ class SymbolDumpVisitor { currStmt_ = std::nullopt; } + bool Pre(const parser::OpenMPDeclareTargetConstruct &x) { + currStmt_ = x.source; + return true; + } + void Post(const parser::OpenMPDeclareTargetConstruct &) { + currStmt_ = std::nullopt; + } + private: std::optional currStmt_; // current statement we are processing std::multimap symbols_; // location to symbol diff --git a/flang/test/Lower/OpenMP/declare-target-func-and-subr.f90 b/flang/test/Lower/OpenMP/declare-target-func-and-subr.f90 index db8320a598052..1c43f1d09eddb 100644 --- a/flang/test/Lower/OpenMP/declare-target-func-and-subr.f90 +++ b/flang/test/Lower/OpenMP/declare-target-func-and-subr.f90 @@ -85,6 +85,13 @@ FUNCTION FUNC_DEFAULT_EXTENDEDLIST() RESULT(I) I = 1 END FUNCTION FUNC_DEFAULT_EXTENDEDLIST +! ALL-LABEL: func.func @_QPfunc_name_as_result() +! ALL-SAME: {{.*}}attributes {omp.declare_target = #omp.declaretarget{{.*}} +FUNCTION FUNC_NAME_AS_RESULT() +!$omp declare target(FUNC_NAME_AS_RESULT) + FUNC_NAME_AS_RESULT = 1.0 +END FUNCTION FUNC_NAME_AS_RESULT + !! ----- ! Check specification valid forms of declare target with subroutines diff --git a/flang/test/Semantics/OpenMP/declare-target-function-name-with-symbols.f90 b/flang/test/Semantics/OpenMP/declare-target-function-name-with-symbols.f90 new file mode 100644 index 0000000000000..9a0acdb3dd100 --- /dev/null +++ b/flang/test/Semantics/OpenMP/declare-target-function-name-with-symbols.f90 @@ -0,0 +1,34 @@ +!RUN: %flang_fc1 -fdebug-unparse-with-symbols -fopenmp %s 2>&1 | FileCheck %s + +! This used to crash. + +module test + contains + function ex(a, b, c) + !$omp declare target(ex) + integer :: a, b, c + ex = a + b + c + end function ex +end module test + +!CHECK: !DEF: /test Module +!CHECK: module test +!CHECK: contains +!CHECK: !DEF: /test/ex PUBLIC (Function, OmpDeclareTarget) Subprogram REAL(4) +!CHECK: !DEF: /test/ex/a ObjectEntity INTEGER(4) +!CHECK: !DEF: /test/ex/b ObjectEntity INTEGER(4) +!CHECK: !DEF: /test/ex/c ObjectEntity INTEGER(4) +!CHECK: function ex(a, b, c) +!CHECK: !$omp declare target (ex) +!CHECK: !REF: /test/ex/a +!CHECK: !REF: /test/ex/b +!CHECK: !REF: /test/ex/c +!CHECK: integer a, b, c +!CHECK: !DEF: /test/ex/ex (Implicit, OmpDeclareTarget) ObjectEntity REAL(4) +!CHECK: !REF: /test/ex/a +!CHECK: !REF: /test/ex/b +!CHECK: !REF: /test/ex/c +!CHECK: ex = a+b+c +!CHECK: end function ex +!CHECK: end module test + From c095c845419a83417928cce4892f924cf693e3be Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 2 Apr 2025 11:11:34 -0500 Subject: [PATCH 2/5] formatting --- flang/lib/Semantics/resolve-directives.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 4ce43e32f4447..db6bbe81fc587 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -2516,8 +2516,8 @@ void OmpAttributeVisitor::ResolveOmpObject( } if (ompFlag == Symbol::Flag::OmpDeclareTarget) { if (symbol->IsFuncResult()) { - if (Scope *container{&currScope()}) { - if (Symbol *func{container->symbol()}) { + if (Scope * container{&currScope()}) { + if (Symbol * func{container->symbol()}) { func->set(ompFlag); name->symbol = func; } From 3fbc03c7e42825772e6d0cd02712080a14d41eca Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 2 Apr 2025 11:57:34 -0500 Subject: [PATCH 3/5] Improve the fix --- flang/lib/Semantics/resolve-directives.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index db6bbe81fc587..ba78d304c3f12 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -2516,11 +2516,11 @@ void OmpAttributeVisitor::ResolveOmpObject( } if (ompFlag == Symbol::Flag::OmpDeclareTarget) { if (symbol->IsFuncResult()) { - if (Scope * container{&currScope()}) { - if (Symbol * func{container->symbol()}) { - func->set(ompFlag); - name->symbol = func; - } + if (Symbol * func{currScope().symbol()}) { + assert(func->IsSubprogram() && + "Expecting function scope"); + func->set(ompFlag); + name->symbol = func; } } } From 15701ee661932cae75b5e92beac421dfe981fadf Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 2 Apr 2025 11:58:57 -0500 Subject: [PATCH 4/5] format --- flang/lib/Semantics/resolve-directives.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index ba78d304c3f12..9f7cc752b436e 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -2517,8 +2517,8 @@ void OmpAttributeVisitor::ResolveOmpObject( if (ompFlag == Symbol::Flag::OmpDeclareTarget) { if (symbol->IsFuncResult()) { if (Symbol * func{currScope().symbol()}) { - assert(func->IsSubprogram() && - "Expecting function scope"); + assert( + func->IsSubprogram() && "Expecting function scope"); func->set(ompFlag); name->symbol = func; } From d021da71e3b9f59222f04303104c2a30ce5cf64e Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 2 Apr 2025 12:28:15 -0500 Subject: [PATCH 5/5] Use CHECK instead of assert --- flang/lib/Semantics/resolve-directives.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 9f7cc752b436e..a5b3391859500 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -2517,8 +2517,7 @@ void OmpAttributeVisitor::ResolveOmpObject( if (ompFlag == Symbol::Flag::OmpDeclareTarget) { if (symbol->IsFuncResult()) { if (Symbol * func{currScope().symbol()}) { - assert( - func->IsSubprogram() && "Expecting function scope"); + CHECK(func->IsSubprogram()); func->set(ompFlag); name->symbol = func; }