Skip to content

[flang][OpenMP] catch namelist access through equivalence #130804

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

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

tblah
Copy link
Contributor

@tblah tblah commented Mar 11, 2025

The standard prohibits privatising namelist variables. We also decided in #110671 to prohibit reductions of namelist variables.

This commit prevents this rule from being circumvented through the use of equivalence statements.

Fixes #122824

The standard prohibits privatising namelist variables. We also decided
in llvm#110671 to prohibit reductions of namelist variables.

This commit prevents this rule from being circumvented through the use
of equivalence statements.

Fixes llvm#122824
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics labels Mar 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 11, 2025

@llvm/pr-subscribers-flang-semantics

Author: Tom Eccles (tblah)

Changes

The standard prohibits privatising namelist variables. We also decided in #110671 to prohibit reductions of namelist variables.

This commit prevents this rule from being circumvented through the use of equivalence statements.

Fixes #122824


Full diff: https://github.com/llvm/llvm-project/pull/130804.diff

2 Files Affected:

  • (modified) flang/lib/Semantics/resolve-directives.cpp (+20-3)
  • (added) flang/test/Semantics/OpenMP/equivalence-namelist.f90 (+32)
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 977c2fef34091..ce96eb9b7782e 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2404,6 +2404,24 @@ void OmpAttributeVisitor::ResolveOmpObjectList(
   }
 }
 
+/// True if either symbol is in a namelist or some other symbol in the same
+/// equivalence set as symbol is in a namelist.
+static bool SymbolOrEquivalentIsInNamelist(const Symbol &symbol) {
+  auto isInNamelist{[](const Symbol &sym) {
+    const Symbol &ultimate{sym.GetUltimate()};
+    return ultimate.test(Symbol::Flag::InNamelist);
+  }};
+
+  const EquivalenceSet *eqv{FindEquivalenceSet(symbol)};
+  if (!eqv) {
+    return isInNamelist(symbol);
+  }
+
+  return llvm::any_of(*eqv, [isInNamelist](const EquivalenceObject &obj) {
+    return isInNamelist(obj.symbol);
+  });
+}
+
 void OmpAttributeVisitor::ResolveOmpObject(
     const parser::OmpObject &ompObject, Symbol::Flag ompFlag) {
   common::visit(
@@ -2468,7 +2486,6 @@ void OmpAttributeVisitor::ResolveOmpObject(
                               .str()));
                 }
                 if (ompFlag == Symbol::Flag::OmpReduction) {
-                  const Symbol &ultimateSymbol{symbol->GetUltimate()};
                   // Using variables inside of a namelist in OpenMP reductions
                   // is allowed by the standard, but is not allowed for
                   // privatisation. This looks like an oversight. If the
@@ -2476,7 +2493,7 @@ void OmpAttributeVisitor::ResolveOmpObject(
                   // mapping for the reduction variable: resulting in incorrect
                   // results. Disabling this hoisting could make some real
                   // production code go slower. See discussion in #109303
-                  if (ultimateSymbol.test(Symbol::Flag::InNamelist)) {
+                  if (SymbolOrEquivalentIsInNamelist(*symbol)) {
                     context_.Say(name->source,
                         "Variable '%s' in NAMELIST cannot be in a REDUCTION clause"_err_en_US,
                         name->ToString());
@@ -2838,7 +2855,7 @@ void OmpAttributeVisitor::CheckObjectIsPrivatizable(
     clauseName = "LASTPRIVATE";
   }
 
-  if (ultimateSymbol.test(Symbol::Flag::InNamelist)) {
+  if (SymbolOrEquivalentIsInNamelist(symbol)) {
     context_.Say(name.source,
         "Variable '%s' in NAMELIST cannot be in a %s clause"_err_en_US,
         name.ToString(), clauseName.str());
diff --git a/flang/test/Semantics/OpenMP/equivalence-namelist.f90 b/flang/test/Semantics/OpenMP/equivalence-namelist.f90
new file mode 100644
index 0000000000000..dc0a1712ed8f5
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/equivalence-namelist.f90
@@ -0,0 +1,32 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+
+! The openmp standard only dissallows namelist for privatization, but flang
+! also does not allow it for reduction as this would be difficult to support.
+!
+! Variables in equivalence with variables in the namelist pose the same
+! implementation problems.
+
+subroutine test01()
+  integer::va
+  equivalence (va,vva)
+  namelist /na1/vva
+  va=1
+
+!ERROR: Variable 'va' in NAMELIST cannot be in a REDUCTION clause
+!$omp parallel reduction(+:va)
+  write(*,na1)
+!$omp end parallel
+end subroutine test01
+
+
+subroutine test02()
+  integer::va
+  equivalence (va,vva)
+  namelist /na1/vva
+  va=1
+
+!ERROR: Variable 'va' in NAMELIST cannot be in a PRIVATE clause
+!$omp parallel private(va)
+  write(*,na1)
+!$omp end parallel
+end subroutine test02

@llvmbot
Copy link
Member

llvmbot commented Mar 11, 2025

@llvm/pr-subscribers-flang-openmp

Author: Tom Eccles (tblah)

Changes

The standard prohibits privatising namelist variables. We also decided in #110671 to prohibit reductions of namelist variables.

This commit prevents this rule from being circumvented through the use of equivalence statements.

Fixes #122824


Full diff: https://github.com/llvm/llvm-project/pull/130804.diff

2 Files Affected:

  • (modified) flang/lib/Semantics/resolve-directives.cpp (+20-3)
  • (added) flang/test/Semantics/OpenMP/equivalence-namelist.f90 (+32)
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 977c2fef34091..ce96eb9b7782e 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2404,6 +2404,24 @@ void OmpAttributeVisitor::ResolveOmpObjectList(
   }
 }
 
+/// True if either symbol is in a namelist or some other symbol in the same
+/// equivalence set as symbol is in a namelist.
+static bool SymbolOrEquivalentIsInNamelist(const Symbol &symbol) {
+  auto isInNamelist{[](const Symbol &sym) {
+    const Symbol &ultimate{sym.GetUltimate()};
+    return ultimate.test(Symbol::Flag::InNamelist);
+  }};
+
+  const EquivalenceSet *eqv{FindEquivalenceSet(symbol)};
+  if (!eqv) {
+    return isInNamelist(symbol);
+  }
+
+  return llvm::any_of(*eqv, [isInNamelist](const EquivalenceObject &obj) {
+    return isInNamelist(obj.symbol);
+  });
+}
+
 void OmpAttributeVisitor::ResolveOmpObject(
     const parser::OmpObject &ompObject, Symbol::Flag ompFlag) {
   common::visit(
@@ -2468,7 +2486,6 @@ void OmpAttributeVisitor::ResolveOmpObject(
                               .str()));
                 }
                 if (ompFlag == Symbol::Flag::OmpReduction) {
-                  const Symbol &ultimateSymbol{symbol->GetUltimate()};
                   // Using variables inside of a namelist in OpenMP reductions
                   // is allowed by the standard, but is not allowed for
                   // privatisation. This looks like an oversight. If the
@@ -2476,7 +2493,7 @@ void OmpAttributeVisitor::ResolveOmpObject(
                   // mapping for the reduction variable: resulting in incorrect
                   // results. Disabling this hoisting could make some real
                   // production code go slower. See discussion in #109303
-                  if (ultimateSymbol.test(Symbol::Flag::InNamelist)) {
+                  if (SymbolOrEquivalentIsInNamelist(*symbol)) {
                     context_.Say(name->source,
                         "Variable '%s' in NAMELIST cannot be in a REDUCTION clause"_err_en_US,
                         name->ToString());
@@ -2838,7 +2855,7 @@ void OmpAttributeVisitor::CheckObjectIsPrivatizable(
     clauseName = "LASTPRIVATE";
   }
 
-  if (ultimateSymbol.test(Symbol::Flag::InNamelist)) {
+  if (SymbolOrEquivalentIsInNamelist(symbol)) {
     context_.Say(name.source,
         "Variable '%s' in NAMELIST cannot be in a %s clause"_err_en_US,
         name.ToString(), clauseName.str());
diff --git a/flang/test/Semantics/OpenMP/equivalence-namelist.f90 b/flang/test/Semantics/OpenMP/equivalence-namelist.f90
new file mode 100644
index 0000000000000..dc0a1712ed8f5
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/equivalence-namelist.f90
@@ -0,0 +1,32 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+
+! The openmp standard only dissallows namelist for privatization, but flang
+! also does not allow it for reduction as this would be difficult to support.
+!
+! Variables in equivalence with variables in the namelist pose the same
+! implementation problems.
+
+subroutine test01()
+  integer::va
+  equivalence (va,vva)
+  namelist /na1/vva
+  va=1
+
+!ERROR: Variable 'va' in NAMELIST cannot be in a REDUCTION clause
+!$omp parallel reduction(+:va)
+  write(*,na1)
+!$omp end parallel
+end subroutine test01
+
+
+subroutine test02()
+  integer::va
+  equivalence (va,vva)
+  namelist /na1/vva
+  va=1
+
+!ERROR: Variable 'va' in NAMELIST cannot be in a PRIVATE clause
+!$omp parallel private(va)
+  write(*,na1)
+!$omp end parallel
+end subroutine test02

Copy link
Contributor

@luporl luporl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@tblah tblah merged commit c851ee3 into llvm:main Mar 12, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:openmp flang:semantics flang Flang issues not falling into any other category
Projects
None yet
3 participants