[flang-commits] [flang] [Flang][OpenMP] Add Semantic Checks for Atomic Capture Construct (PR #108516)

via flang-commits flang-commits at lists.llvm.org
Tue Sep 24 03:25:33 PDT 2024


https://github.com/harishch4 updated https://github.com/llvm/llvm-project/pull/108516

>From bbd04979f59a24bd03579250d31bf6616fe0a341 Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Fri, 13 Sep 2024 14:07:41 +0530
Subject: [PATCH 1/8] [Flang][OpenMP] Add Semantic Checks for Atomic Capture
 Construct

---
 flang/include/flang/Semantics/tools.h         | 27 +++++++
 flang/lib/Lower/DirectivesCommon.h            | 34 +--------
 flang/lib/Semantics/check-omp-structure.cpp   | 76 +++++++++++++++++--
 flang/lib/Semantics/check-omp-structure.h     |  1 +
 .../OpenMP/omp-atomic-assignment-stmt.f90     | 40 ++++++++++
 .../Semantics/OpenMP/requires-atomic01.f90    |  6 +-
 .../Semantics/OpenMP/requires-atomic02.f90    |  6 +-
 7 files changed, 147 insertions(+), 43 deletions(-)

diff --git a/flang/include/flang/Semantics/tools.h b/flang/include/flang/Semantics/tools.h
index 15c02ecc0058cc..b60514b991b727 100644
--- a/flang/include/flang/Semantics/tools.h
+++ b/flang/include/flang/Semantics/tools.h
@@ -736,5 +736,32 @@ std::string GetCommonBlockObjectName(const Symbol &, bool underscoring);
 // Check for ambiguous USE associations
 bool HadUseError(SemanticsContext &, SourceName at, const Symbol *);
 
+/// Checks if the assignment statement has a single variable on the RHS.
+inline bool checkForSingleVariableOnRHS(
+    const Fortran::parser::AssignmentStmt &assignmentStmt) {
+  const Fortran::parser::Expr &expr{
+      std::get<Fortran::parser::Expr>(assignmentStmt.t)};
+  const Fortran::common::Indirection<Fortran::parser::Designator> *designator =
+      std::get_if<Fortran::common::Indirection<Fortran::parser::Designator>>(
+          &expr.u);
+  return designator != nullptr;
+}
+
+/// Checks if the symbol on the LHS of the assignment statement is present in
+/// the RHS expression.
+inline bool checkForSymbolMatch(
+    const Fortran::parser::AssignmentStmt &assignmentStmt) {
+  const auto &var{std::get<Fortran::parser::Variable>(assignmentStmt.t)};
+  const auto &expr{std::get<Fortran::parser::Expr>(assignmentStmt.t)};
+  const auto *e{Fortran::semantics::GetExpr(expr)};
+  const auto *v{Fortran::semantics::GetExpr(var)};
+  auto varSyms{Fortran::evaluate::GetSymbolVector(*v)};
+  const Fortran::semantics::Symbol &varSymbol{*varSyms.front()};
+  for (const Fortran::semantics::Symbol &symbol :
+      Fortran::evaluate::GetSymbolVector(*e))
+    if (varSymbol == symbol)
+      return true;
+  return false;
+}
 } // namespace Fortran::semantics
 #endif // FORTRAN_SEMANTICS_TOOLS_H_
diff --git a/flang/lib/Lower/DirectivesCommon.h b/flang/lib/Lower/DirectivesCommon.h
index d2060e77ce5305..a32f0b287e049a 100644
--- a/flang/lib/Lower/DirectivesCommon.h
+++ b/flang/lib/Lower/DirectivesCommon.h
@@ -74,34 +74,6 @@ struct AddrAndBoundsInfo {
   }
 };
 
-/// Checks if the assignment statement has a single variable on the RHS.
-static inline bool checkForSingleVariableOnRHS(
-    const Fortran::parser::AssignmentStmt &assignmentStmt) {
-  const Fortran::parser::Expr &expr{
-      std::get<Fortran::parser::Expr>(assignmentStmt.t)};
-  const Fortran::common::Indirection<Fortran::parser::Designator> *designator =
-      std::get_if<Fortran::common::Indirection<Fortran::parser::Designator>>(
-          &expr.u);
-  return designator != nullptr;
-}
-
-/// Checks if the symbol on the LHS of the assignment statement is present in
-/// the RHS expression.
-static inline bool
-checkForSymbolMatch(const Fortran::parser::AssignmentStmt &assignmentStmt) {
-  const auto &var{std::get<Fortran::parser::Variable>(assignmentStmt.t)};
-  const auto &expr{std::get<Fortran::parser::Expr>(assignmentStmt.t)};
-  const auto *e{Fortran::semantics::GetExpr(expr)};
-  const auto *v{Fortran::semantics::GetExpr(var)};
-  auto varSyms{Fortran::evaluate::GetSymbolVector(*v)};
-  const Fortran::semantics::Symbol &varSymbol{*varSyms.front()};
-  for (const Fortran::semantics::Symbol &symbol :
-       Fortran::evaluate::GetSymbolVector(*e))
-    if (varSymbol == symbol)
-      return true;
-  return false;
-}
-
 /// Populates \p hint and \p memoryOrder with appropriate clause information
 /// if present on atomic construct.
 static inline void genOmpAtomicHintAndMemoryOrderClauses(
@@ -537,7 +509,7 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
   stmt2LHSArg = fir::getBase(converter.genExprAddr(assign2.lhs, stmtCtx));
 
   // Operation specific RHS evaluations
-  if (checkForSingleVariableOnRHS(stmt1)) {
+  if (Fortran::semantics::checkForSingleVariableOnRHS(stmt1)) {
     // Atomic capture construct is of the form [capture-stmt, update-stmt] or
     // of the form [capture-stmt, write-stmt]
     stmt1RHSArg = fir::getBase(converter.genExprAddr(assign1.rhs, stmtCtx));
@@ -573,8 +545,8 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
   firOpBuilder.createBlock(&(atomicCaptureOp->getRegion(0)));
   mlir::Block &block = atomicCaptureOp->getRegion(0).back();
   firOpBuilder.setInsertionPointToStart(&block);
-  if (checkForSingleVariableOnRHS(stmt1)) {
-    if (checkForSymbolMatch(stmt2)) {
+  if (Fortran::semantics::checkForSingleVariableOnRHS(stmt1)) {
+    if (Fortran::semantics::checkForSymbolMatch(stmt2)) {
       // Atomic capture construct is of the form [capture-stmt, update-stmt]
       const Fortran::semantics::SomeExpr &fromExpr =
           *Fortran::semantics::GetExpr(stmt1Expr);
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 643b713b32e29d..662e6c10bd4e03 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1977,6 +1977,70 @@ void OmpStructureChecker::CheckAtomicUpdateStmt(
   ErrIfAllocatableVariable(var);
 }
 
+void OmpStructureChecker::CheckAtomicCaptureConstruct(
+    const parser::OmpAtomicCapture &atomicCaptureConstruct) {
+  const Fortran::parser::AssignmentStmt &stmt1 =
+      std::get<Fortran::parser::OmpAtomicCapture::Stmt1>(
+          atomicCaptureConstruct.t)
+          .v.statement;
+  const auto &stmt1Var{std::get<Fortran::parser::Variable>(stmt1.t)};
+  const auto &stmt1Expr{std::get<Fortran::parser::Expr>(stmt1.t)};
+
+  const Fortran::parser::AssignmentStmt &stmt2 =
+      std::get<Fortran::parser::OmpAtomicCapture::Stmt2>(
+          atomicCaptureConstruct.t)
+          .v.statement;
+  const auto &stmt2Var{std::get<Fortran::parser::Variable>(stmt2.t)};
+  const auto &stmt2Expr{std::get<Fortran::parser::Expr>(stmt2.t)};
+
+  if (Fortran::semantics::checkForSingleVariableOnRHS(stmt1)) {
+    CheckAtomicCaptureStmt(stmt1);
+    if (Fortran::semantics::checkForSymbolMatch(stmt2)) {
+      // Atomic capture construct is of the form [capture-stmt, update-stmt]
+      CheckAtomicUpdateStmt(stmt2);
+    } else {
+      // Atomic capture construct is of the form [capture-stmt, write-stmt]
+      CheckAtomicWriteStmt(stmt2);
+    }
+    // Variable captured in stmt1 should be assigned in stmt2
+    const auto *e{GetExpr(context_, stmt1Expr)};
+    const auto *v{GetExpr(context_, stmt2Var)};
+    if (e && v) {
+      const Symbol &stmt2VarSymbol = evaluate::GetSymbolVector(*v).front();
+      const Symbol &stmt1ExprSymbol = evaluate::GetSymbolVector(*e).front();
+      if (stmt2VarSymbol != stmt1ExprSymbol)
+        context_.Say(stmt1Expr.source,
+            "Captured variable %s "
+            "expected to be assigned in statement 2 of "
+            "atomic capture construct"_err_en_US,
+            stmt1ExprSymbol.name().ToString());
+    }
+  } else if (Fortran::semantics::checkForSymbolMatch(stmt1) &&
+      Fortran::semantics::checkForSingleVariableOnRHS(stmt2)) {
+    // Atomic capture construct is of the form [update-stmt, capture-stmt]
+    CheckAtomicUpdateStmt(stmt1);
+    CheckAtomicCaptureStmt(stmt2);
+    // Variable updated in stmt1 should be captured in stmt2
+    const auto *e{GetExpr(context_, stmt2Expr)};
+    const auto *v{GetExpr(context_, stmt1Var)};
+    if (e && v) {
+      const Symbol &stmt1VarSymbol = evaluate::GetSymbolVector(*v).front();
+      const Symbol &stmt2ExprSymbol = evaluate::GetSymbolVector(*e).front();
+      if (stmt1VarSymbol != stmt2ExprSymbol)
+        context_.Say(stmt1Var.GetSource(),
+            "Updated variable %s "
+            "expected to be captured in statement 2 of "
+            "atomic capture construct"_err_en_US,
+            stmt1Var.GetSource().ToString());
+    }
+  } else {
+    context_.Say(stmt1Expr.source,
+        "Invalid atomic capture construct statements. "
+        "Expected one of [update-stmt, capture-stmt], "
+        "[capture-stmt, update-stmt], or [capture-stmt, write-stmt]"_err_en_US);
+  }
+}
+
 void OmpStructureChecker::CheckAtomicMemoryOrderClause(
     const parser::OmpAtomicClauseList *leftHandClauseList,
     const parser::OmpAtomicClauseList *rightHandClauseList) {
@@ -2060,15 +2124,15 @@ void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x) {
                     atomicWrite.t)
                     .statement);
           },
-          [&](const auto &atomicConstruct) {
-            const auto &dir{std::get<parser::Verbatim>(atomicConstruct.t)};
+          [&](const parser::OmpAtomicCapture &atomicCapture) {
+            const auto &dir{std::get<parser::Verbatim>(atomicCapture.t)};
             PushContextAndClauseSets(
                 dir.source, llvm::omp::Directive::OMPD_atomic);
-            CheckAtomicMemoryOrderClause(&std::get<0>(atomicConstruct.t),
-                &std::get<2>(atomicConstruct.t));
+            CheckAtomicMemoryOrderClause(
+                &std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t));
             CheckHintClause<const parser::OmpAtomicClauseList>(
-                &std::get<0>(atomicConstruct.t),
-                &std::get<2>(atomicConstruct.t));
+                &std::get<0>(atomicCapture.t), &std::get<2>(atomicCapture.t));
+            CheckAtomicCaptureConstruct(atomicCapture);
           },
       },
       x.u);
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 2cc1a78068f540..8bfd4d594b028e 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -193,6 +193,7 @@ class OmpStructureChecker
   void CheckAtomicUpdateStmt(const parser::AssignmentStmt &);
   void CheckAtomicCaptureStmt(const parser::AssignmentStmt &);
   void CheckAtomicWriteStmt(const parser::AssignmentStmt &);
+  void CheckAtomicCaptureConstruct(const parser::OmpAtomicCapture &);
   void CheckAtomicConstructStructure(const parser::OpenMPAtomicConstruct &);
   void CheckDistLinear(const parser::OpenMPLoopConstruct &x);
   void CheckSIMDNest(const parser::OpenMPConstruct &x);
diff --git a/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90 b/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
index a346056dee383b..09921944e7fc06 100644
--- a/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
+++ b/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
@@ -84,4 +84,44 @@ program sample
     !$omp atomic write
     !ERROR: Expected scalar variable on the LHS of atomic assignment statement
         a = x
+
+    !$omp atomic capture
+        v = x
+        x = x + 1
+    !$omp end atomic
+
+    !$omp atomic release capture
+        v = x
+    !ERROR: Atomic update statement should be of form `x = x operator expr` OR `x = expr operator x`
+        x = b + (x*1)
+    !$omp end atomic
+
+    !$omp atomic capture hint(0)
+        v = x
+        x = 1
+    !$omp end atomic
+
+    !$omp atomic capture
+    !ERROR: Captured variable x expected to be assigned in statement 2 of atomic capture construct
+        v = x
+        b = b + 1
+    !$omp end atomic
+
+    !$omp atomic capture
+    !ERROR: Captured variable x expected to be assigned in statement 2 of atomic capture construct
+        v = x
+        b = 10
+    !$omp end atomic
+
+    !$omp atomic capture
+    !ERROR: Updated variable x expected to be captured in statement 2 of atomic capture construct
+        x = x + 10
+        v = b
+    !$omp end atomic
+
+    !$omp atomic capture
+    !ERROR: Invalid atomic capture construct statements. Expected one of [update-stmt, capture-stmt], [capture-stmt, update-stmt], or [capture-stmt, write-stmt]
+        v = 1
+        x = 4
+    !$omp end atomic
 end program
diff --git a/flang/test/Semantics/OpenMP/requires-atomic01.f90 b/flang/test/Semantics/OpenMP/requires-atomic01.f90
index b39c9cdcc0bb33..cb7b1bc1ac52ab 100644
--- a/flang/test/Semantics/OpenMP/requires-atomic01.f90
+++ b/flang/test/Semantics/OpenMP/requires-atomic01.f90
@@ -88,7 +88,7 @@ program requires
   ! CHECK: OmpMemoryOrderClause -> OmpClause -> SeqCst
   !$omp atomic capture
   i = j
-  i = j
+  j = j + 1
   !$omp end atomic
 
   ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
@@ -96,7 +96,7 @@ program requires
   ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
   !$omp atomic relaxed capture
   i = j
-  i = j
+  j = j + 1
   !$omp end atomic
 
   ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
@@ -104,6 +104,6 @@ program requires
   ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
   !$omp atomic capture relaxed
   i = j
-  i = j
+  j = j + 1
   !$omp end atomic
 end program requires
diff --git a/flang/test/Semantics/OpenMP/requires-atomic02.f90 b/flang/test/Semantics/OpenMP/requires-atomic02.f90
index 3af83970e7927a..5a4249794f7b50 100644
--- a/flang/test/Semantics/OpenMP/requires-atomic02.f90
+++ b/flang/test/Semantics/OpenMP/requires-atomic02.f90
@@ -88,7 +88,7 @@ program requires
   ! CHECK: OmpMemoryOrderClause -> OmpClause -> AcqRel
   !$omp atomic capture
   i = j
-  i = j
+  j = j + 1
   !$omp end atomic
 
   ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
@@ -96,7 +96,7 @@ program requires
   ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
   !$omp atomic relaxed capture
   i = j
-  i = j
+  j = j + 1
   !$omp end atomic
 
   ! CHECK-LABEL: OpenMPAtomicConstruct -> OmpAtomicCapture
@@ -104,6 +104,6 @@ program requires
   ! CHECK: OmpMemoryOrderClause -> OmpClause -> Relaxed
   !$omp atomic capture relaxed
   i = j
-  i = j
+  j = j + 1
   !$omp end atomic
 end program requires

>From 2b4292126927ffcbe86fd462257e70a79ecd9feb Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Tue, 17 Sep 2024 15:29:23 +0530
Subject: [PATCH 2/8] Add TODO

---
 flang/lib/Semantics/check-omp-structure.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 662e6c10bd4e03..8365793d473374 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1977,6 +1977,7 @@ void OmpStructureChecker::CheckAtomicUpdateStmt(
   ErrIfAllocatableVariable(var);
 }
 
+// TODO: Allow cond-update-stmt once compare clause is supported.
 void OmpStructureChecker::CheckAtomicCaptureConstruct(
     const parser::OmpAtomicCapture &atomicCaptureConstruct) {
   const Fortran::parser::AssignmentStmt &stmt1 =

>From 3e3eb13b267427b07ef57007c2e230dd6ed0a449 Mon Sep 17 00:00:00 2001
From: harishch4 <harishcse44 at gmail.com>
Date: Wed, 18 Sep 2024 17:33:34 +0530
Subject: [PATCH 3/8] Update flang/lib/Semantics/check-omp-structure.cpp

Co-authored-by: Kiran Chandramohan <kiranchandramohan at gmail.com>
---
 flang/lib/Semantics/check-omp-structure.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 8365793d473374..f8fc3c9727a304 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2030,7 +2030,7 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
       if (stmt1VarSymbol != stmt2ExprSymbol)
         context_.Say(stmt1Var.GetSource(),
             "Updated variable %s "
-            "expected to be captured in statement 2 of "
+            "expected to be captured in the second statement of "
             "atomic capture construct"_err_en_US,
             stmt1Var.GetSource().ToString());
     }

>From a7e38da538b313939ec52030a4585e35889d7350 Mon Sep 17 00:00:00 2001
From: harishch4 <harishcse44 at gmail.com>
Date: Wed, 18 Sep 2024 17:33:45 +0530
Subject: [PATCH 4/8] Update flang/lib/Semantics/check-omp-structure.cpp

Co-authored-by: Kiran Chandramohan <kiranchandramohan at gmail.com>
---
 flang/lib/Semantics/check-omp-structure.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index f8fc3c9727a304..2893859ae99152 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2012,7 +2012,7 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
       if (stmt2VarSymbol != stmt1ExprSymbol)
         context_.Say(stmt1Expr.source,
             "Captured variable %s "
-            "expected to be assigned in statement 2 of "
+            "expected to be assigned in the second statement of "
             "atomic capture construct"_err_en_US,
             stmt1ExprSymbol.name().ToString());
     }

>From 1a76168b7cded0ef7cbef1d572fabaaa1eaf89be Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Wed, 18 Sep 2024 17:40:12 +0530
Subject: [PATCH 5/8] Add braces

---
 flang/include/flang/Semantics/tools.h       | 6 ++++--
 flang/lib/Semantics/check-omp-structure.cpp | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/flang/include/flang/Semantics/tools.h b/flang/include/flang/Semantics/tools.h
index b60514b991b727..96d4dbb2acaa11 100644
--- a/flang/include/flang/Semantics/tools.h
+++ b/flang/include/flang/Semantics/tools.h
@@ -758,9 +758,11 @@ inline bool checkForSymbolMatch(
   auto varSyms{Fortran::evaluate::GetSymbolVector(*v)};
   const Fortran::semantics::Symbol &varSymbol{*varSyms.front()};
   for (const Fortran::semantics::Symbol &symbol :
-      Fortran::evaluate::GetSymbolVector(*e))
-    if (varSymbol == symbol)
+      Fortran::evaluate::GetSymbolVector(*e)) {
+    if (varSymbol == symbol) {
       return true;
+    }
+  }
   return false;
 }
 } // namespace Fortran::semantics
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 2893859ae99152..18b8ca5f511771 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2009,12 +2009,13 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
     if (e && v) {
       const Symbol &stmt2VarSymbol = evaluate::GetSymbolVector(*v).front();
       const Symbol &stmt1ExprSymbol = evaluate::GetSymbolVector(*e).front();
-      if (stmt2VarSymbol != stmt1ExprSymbol)
+      if (stmt2VarSymbol != stmt1ExprSymbol) {
         context_.Say(stmt1Expr.source,
             "Captured variable %s "
             "expected to be assigned in the second statement of "
             "atomic capture construct"_err_en_US,
             stmt1ExprSymbol.name().ToString());
+      }
     }
   } else if (Fortran::semantics::checkForSymbolMatch(stmt1) &&
       Fortran::semantics::checkForSingleVariableOnRHS(stmt2)) {

>From 9e7cbf4c0aa7d3443119c68908b7a524598f29b3 Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Thu, 19 Sep 2024 15:54:24 +0530
Subject: [PATCH 6/8] Address review comments

---
 flang/lib/Semantics/check-omp-structure.cpp                | 4 ++--
 flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90 | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 18b8ca5f511771..49cfcef871b5da 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2014,7 +2014,7 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
             "Captured variable %s "
             "expected to be assigned in the second statement of "
             "atomic capture construct"_err_en_US,
-            stmt1ExprSymbol.name().ToString());
+            stmt1ExprSymbol.name());
       }
     }
   } else if (Fortran::semantics::checkForSymbolMatch(stmt1) &&
@@ -2033,7 +2033,7 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
             "Updated variable %s "
             "expected to be captured in the second statement of "
             "atomic capture construct"_err_en_US,
-            stmt1Var.GetSource().ToString());
+            stmt1Var.GetSource());
     }
   } else {
     context_.Say(stmt1Expr.source,
diff --git a/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90 b/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
index 09921944e7fc06..aba7adb3d34cf0 100644
--- a/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
+++ b/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
@@ -102,19 +102,19 @@ program sample
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Captured variable x expected to be assigned in statement 2 of atomic capture construct
+    !ERROR: Captured variable x expected to be assigned in the second statement of atomic capture construct
         v = x
         b = b + 1
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Captured variable x expected to be assigned in statement 2 of atomic capture construct
+    !ERROR: Captured variable x expected to be assigned in the second statement of atomic capture construct
         v = x
         b = 10
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Updated variable x expected to be captured in statement 2 of atomic capture construct
+    !ERROR: Updated variable x expected to be captured in the second statement of atomic capture construct
         x = x + 10
         v = b
     !$omp end atomic

>From 87aa33268b26d2fcdec3057f21378b0ca8c6031f Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Thu, 19 Sep 2024 17:24:38 +0530
Subject: [PATCH 7/8] replace symbol check with typedExpr check

---
 flang/lib/Semantics/check-omp-structure.cpp   | 40 ++++++++-----------
 .../OpenMP/omp-atomic-assignment-stmt.f90     | 24 +++++++++++
 2 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 49cfcef871b5da..30b20b155c1710 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2003,19 +2003,14 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
       // Atomic capture construct is of the form [capture-stmt, write-stmt]
       CheckAtomicWriteStmt(stmt2);
     }
-    // Variable captured in stmt1 should be assigned in stmt2
-    const auto *e{GetExpr(context_, stmt1Expr)};
-    const auto *v{GetExpr(context_, stmt2Var)};
-    if (e && v) {
-      const Symbol &stmt2VarSymbol = evaluate::GetSymbolVector(*v).front();
-      const Symbol &stmt1ExprSymbol = evaluate::GetSymbolVector(*e).front();
-      if (stmt2VarSymbol != stmt1ExprSymbol) {
-        context_.Say(stmt1Expr.source,
-            "Captured variable %s "
-            "expected to be assigned in the second statement of "
-            "atomic capture construct"_err_en_US,
-            stmt1ExprSymbol.name());
-      }
+    auto *v{stmt2Var.typedExpr.get()};
+    auto *e{stmt1Expr.typedExpr.get()};
+    if (v && e && !(v->v == e->v)) {
+      context_.Say(stmt1Expr.source,
+          "Captured variable %s "
+          "expected to be assigned in the second statement of "
+          "atomic capture construct"_err_en_US,
+          stmt1Expr.source);
     }
   } else if (Fortran::semantics::checkForSymbolMatch(stmt1) &&
       Fortran::semantics::checkForSingleVariableOnRHS(stmt2)) {
@@ -2023,17 +2018,14 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
     CheckAtomicUpdateStmt(stmt1);
     CheckAtomicCaptureStmt(stmt2);
     // Variable updated in stmt1 should be captured in stmt2
-    const auto *e{GetExpr(context_, stmt2Expr)};
-    const auto *v{GetExpr(context_, stmt1Var)};
-    if (e && v) {
-      const Symbol &stmt1VarSymbol = evaluate::GetSymbolVector(*v).front();
-      const Symbol &stmt2ExprSymbol = evaluate::GetSymbolVector(*e).front();
-      if (stmt1VarSymbol != stmt2ExprSymbol)
-        context_.Say(stmt1Var.GetSource(),
-            "Updated variable %s "
-            "expected to be captured in the second statement of "
-            "atomic capture construct"_err_en_US,
-            stmt1Var.GetSource());
+    auto *v{stmt1Var.typedExpr.get()};
+    auto *e{stmt2Expr.typedExpr.get()};
+    if (v && e && !(v->v == e->v)) {
+      context_.Say(stmt1Var.GetSource(),
+          "Updated variable %s "
+          "expected to be captured in the second statement of "
+          "atomic capture construct"_err_en_US,
+          stmt1Var.GetSource());
     }
   } else {
     context_.Say(stmt1Expr.source,
diff --git a/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90 b/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
index aba7adb3d34cf0..d03e5c3a336684 100644
--- a/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
+++ b/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
@@ -124,4 +124,28 @@ program sample
         v = 1
         x = 4
     !$omp end atomic
+
+    !$omp atomic capture
+    !ERROR: Captured variable z%y expected to be assigned in the second statement of atomic capture construct
+        x = z%y
+        z%m = z%m + 1.0
+    !$omp end atomic
+
+    !$omp atomic capture
+    !ERROR: Updated variable z%m expected to be captured in the second statement of atomic capture construct
+        z%m = z%m + 1.0
+        x = z%y
+    !$omp end atomic
+
+    !$omp atomic capture
+    !ERROR: Captured variable y(2) expected to be assigned in the second statement of atomic capture construct
+        x = y(2)
+        y(1) = y(1) + 1
+    !$omp end atomic
+
+    !$omp atomic capture
+    !ERROR: Updated variable y(1) expected to be captured in the second statement of atomic capture construct
+        y(1) = y(1) + 1
+        x = y(2)
+    !$omp end atomic
 end program

>From aa4b64f250692385c6094aa886371dac6c5b83aa Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Tue, 24 Sep 2024 15:55:13 +0530
Subject: [PATCH 8/8] Refactor error messages

---
 flang/lib/Semantics/check-omp-structure.cpp    | 18 ++++++------------
 .../OpenMP/omp-atomic-assignment-stmt.f90      | 16 ++++++++--------
 2 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 30b20b155c1710..dfc3f3290a81be 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1997,24 +1997,22 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
   if (Fortran::semantics::checkForSingleVariableOnRHS(stmt1)) {
     CheckAtomicCaptureStmt(stmt1);
     if (Fortran::semantics::checkForSymbolMatch(stmt2)) {
-      // Atomic capture construct is of the form [capture-stmt, update-stmt]
+      // ATOMIC CAPTURE construct is of the form [capture-stmt, update-stmt]
       CheckAtomicUpdateStmt(stmt2);
     } else {
-      // Atomic capture construct is of the form [capture-stmt, write-stmt]
+      // ATOMIC CAPTURE construct is of the form [capture-stmt, write-stmt]
       CheckAtomicWriteStmt(stmt2);
     }
     auto *v{stmt2Var.typedExpr.get()};
     auto *e{stmt1Expr.typedExpr.get()};
     if (v && e && !(v->v == e->v)) {
       context_.Say(stmt1Expr.source,
-          "Captured variable %s "
-          "expected to be assigned in the second statement of "
-          "atomic capture construct"_err_en_US,
+          "Captured variable/array element/derived-type component %s expected to be assigned in the second statement of ATOMIC CAPTURE construct"_err_en_US,
           stmt1Expr.source);
     }
   } else if (Fortran::semantics::checkForSymbolMatch(stmt1) &&
       Fortran::semantics::checkForSingleVariableOnRHS(stmt2)) {
-    // Atomic capture construct is of the form [update-stmt, capture-stmt]
+    // ATOMIC CAPTURE construct is of the form [update-stmt, capture-stmt]
     CheckAtomicUpdateStmt(stmt1);
     CheckAtomicCaptureStmt(stmt2);
     // Variable updated in stmt1 should be captured in stmt2
@@ -2022,16 +2020,12 @@ void OmpStructureChecker::CheckAtomicCaptureConstruct(
     auto *e{stmt2Expr.typedExpr.get()};
     if (v && e && !(v->v == e->v)) {
       context_.Say(stmt1Var.GetSource(),
-          "Updated variable %s "
-          "expected to be captured in the second statement of "
-          "atomic capture construct"_err_en_US,
+          "Updated variable/array element/derived-type component %s expected to be captured in the second statement of ATOMIC CAPTURE construct"_err_en_US,
           stmt1Var.GetSource());
     }
   } else {
     context_.Say(stmt1Expr.source,
-        "Invalid atomic capture construct statements. "
-        "Expected one of [update-stmt, capture-stmt], "
-        "[capture-stmt, update-stmt], or [capture-stmt, write-stmt]"_err_en_US);
+        "Invalid ATOMIC CAPTURE construct statements. Expected one of [update-stmt, capture-stmt], [capture-stmt, update-stmt], or [capture-stmt, write-stmt]"_err_en_US);
   }
 }
 
diff --git a/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90 b/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
index d03e5c3a336684..0d4da5485af046 100644
--- a/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
+++ b/flang/test/Semantics/OpenMP/omp-atomic-assignment-stmt.f90
@@ -102,49 +102,49 @@ program sample
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Captured variable x expected to be assigned in the second statement of atomic capture construct
+    !ERROR: Captured variable/array element/derived-type component x expected to be assigned in the second statement of ATOMIC CAPTURE construct
         v = x
         b = b + 1
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Captured variable x expected to be assigned in the second statement of atomic capture construct
+    !ERROR: Captured variable/array element/derived-type component x expected to be assigned in the second statement of ATOMIC CAPTURE construct
         v = x
         b = 10
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Updated variable x expected to be captured in the second statement of atomic capture construct
+    !ERROR: Updated variable/array element/derived-type component x expected to be captured in the second statement of ATOMIC CAPTURE construct
         x = x + 10
         v = b
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Invalid atomic capture construct statements. Expected one of [update-stmt, capture-stmt], [capture-stmt, update-stmt], or [capture-stmt, write-stmt]
+    !ERROR: Invalid ATOMIC CAPTURE construct statements. Expected one of [update-stmt, capture-stmt], [capture-stmt, update-stmt], or [capture-stmt, write-stmt]
         v = 1
         x = 4
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Captured variable z%y expected to be assigned in the second statement of atomic capture construct
+    !ERROR: Captured variable/array element/derived-type component z%y expected to be assigned in the second statement of ATOMIC CAPTURE construct
         x = z%y
         z%m = z%m + 1.0
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Updated variable z%m expected to be captured in the second statement of atomic capture construct
+    !ERROR: Updated variable/array element/derived-type component z%m expected to be captured in the second statement of ATOMIC CAPTURE construct
         z%m = z%m + 1.0
         x = z%y
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Captured variable y(2) expected to be assigned in the second statement of atomic capture construct
+    !ERROR: Captured variable/array element/derived-type component y(2) expected to be assigned in the second statement of ATOMIC CAPTURE construct
         x = y(2)
         y(1) = y(1) + 1
     !$omp end atomic
 
     !$omp atomic capture
-    !ERROR: Updated variable y(1) expected to be captured in the second statement of atomic capture construct
+    !ERROR: Updated variable/array element/derived-type component y(1) expected to be captured in the second statement of ATOMIC CAPTURE construct
         y(1) = y(1) + 1
         x = y(2)
     !$omp end atomic



More information about the flang-commits mailing list