[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-06 Thread Tom Eccles via llvm-branch-commits

https://github.com/tblah approved this pull request.

LGTM, thanks

https://github.com/llvm/llvm-project/pull/152007
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-05 Thread Jack Styles via llvm-branch-commits


@@ -34,6 +34,7 @@
 #include "flang/Parser/openmp-utils.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/openmp-directive-sets.h"
+#include "flang/Semantics/openmp-utils.h"

Stylie777 wrote:

Makes sense, thanks.

https://github.com/llvm/llvm-project/pull/152007
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-05 Thread Krzysztof Parzyszek via llvm-branch-commits


@@ -34,6 +34,7 @@
 #include "flang/Parser/openmp-utils.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/openmp-directive-sets.h"
+#include "flang/Semantics/openmp-utils.h"

kparzysz wrote:

Yes, this header was local to lib/Semantics, and I wanted to use it in here in 
Lower, so I moved it to include/flang/Semantics.

https://github.com/llvm/llvm-project/pull/152007
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-05 Thread Jack Styles via llvm-branch-commits

https://github.com/Stylie777 approved this pull request.

LGTM other than one nit comment.

https://github.com/llvm/llvm-project/pull/152007
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-05 Thread Jack Styles via llvm-branch-commits


@@ -34,6 +34,7 @@
 #include "flang/Parser/openmp-utils.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/openmp-directive-sets.h"
+#include "flang/Semantics/openmp-utils.h"

Stylie777 wrote:

nit: Is this change to the header file location critical for this patch? If it 
is not, could we do this in a separate PR? Otherwise, please ignore this 
comment.

https://github.com/llvm/llvm-project/pull/152007
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-05 Thread Jack Styles via llvm-branch-commits

https://github.com/Stylie777 edited 
https://github.com/llvm/llvm-project/pull/152007
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-04 Thread via llvm-branch-commits

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- 
flang/include/flang/Parser/parse-tree.h flang/lib/Lower/OpenMP/OpenMP.cpp 
flang/lib/Parser/openmp-parsers.cpp flang/lib/Parser/unparse.cpp 
flang/lib/Semantics/check-omp-atomic.cpp flang/lib/Semantics/check-omp-loop.cpp 
flang/lib/Semantics/check-omp-metadirective.cpp 
flang/lib/Semantics/check-omp-structure.cpp 
flang/lib/Semantics/openmp-utils.cpp flang/lib/Semantics/resolve-directives.cpp 
flang/lib/Semantics/resolve-names.cpp 
flang/lib/Semantics/unparse-with-symbols.cpp 
flang/include/flang/Semantics/openmp-utils.h
``





View the diff from clang-format here.


``diff
diff --git a/flang/lib/Parser/openmp-parsers.cpp 
b/flang/lib/Parser/openmp-parsers.cpp
index ab23e7d70..46b148610 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1758,8 +1758,8 @@ 
TYPE_PARSER(sourced(construct(
 TYPE_PARSER(construct(Parser{}) ||
 construct(Parser{}))
 
-TYPE_PARSER(construct(OmpBlockConstructParser{
-llvm::omp::Directive::OMPD_critical}))
+TYPE_PARSER(construct(
+OmpBlockConstructParser{llvm::omp::Directive::OMPD_critical}))
 
 // 2.11.3 Executable Allocate directive
 TYPE_PARSER(

``




https://github.com/llvm/llvm-project/pull/152007
___
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-04 Thread Krzysztof Parzyszek via llvm-branch-commits

https://github.com/kparzysz updated 
https://github.com/llvm/llvm-project/pull/152007

>From d57337ec742dd2ace259ab7599b587265723840d Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek 
Date: Mon, 4 Aug 2025 07:59:47 -0500
Subject: [PATCH 1/2] [flang][OpenMP] Make OpenMPCriticalConstruct follow block
 structure

This allows not having the END CRITICAL directive in certain situations.
Update semantic checks and symbol resolution.
---
 flang/include/flang/Parser/parse-tree.h   |   6 +-
 .../flang}/Semantics/openmp-utils.h   |   0
 flang/lib/Lower/OpenMP/OpenMP.cpp |  24 +++-
 flang/lib/Parser/openmp-parsers.cpp   |  13 +-
 flang/lib/Parser/unparse.cpp  |   4 +-
 flang/lib/Semantics/check-omp-atomic.cpp  |   2 +-
 flang/lib/Semantics/check-omp-loop.cpp|   2 +-
 .../lib/Semantics/check-omp-metadirective.cpp |   3 +-
 flang/lib/Semantics/check-omp-structure.cpp   | 124 --
 flang/lib/Semantics/openmp-utils.cpp  |   2 +-
 flang/lib/Semantics/resolve-directives.cpp|   6 +-
 flang/lib/Semantics/resolve-names.cpp |  69 +-
 flang/lib/Semantics/unparse-with-symbols.cpp  |  14 --
 .../OpenMP/critical-unparse-with-symbols.f90  |   4 +-
 .../test/Semantics/OpenMP/sync-critical01.f90 |   8 +-
 .../test/Semantics/OpenMP/sync-critical02.f90 |   8 +-
 16 files changed, 155 insertions(+), 134 deletions(-)
 rename flang/{lib => include/flang}/Semantics/openmp-utils.h (100%)

diff --git a/flang/include/flang/Parser/parse-tree.h 
b/flang/include/flang/Parser/parse-tree.h
index 8302e40984af0..e72190f019dd1 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4986,9 +4986,9 @@ struct OmpEndCriticalDirective {
   CharBlock source;
   std::tuple> t;
 };
-struct OpenMPCriticalConstruct {
-  TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct);
-  std::tuple t;
+
+struct OpenMPCriticalConstruct : public OmpBlockConstruct {
+  INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct, 
OmpBlockConstruct);
 };
 
 // 2.11.3 allocate -> ALLOCATE [(variable-name-list)] [clause]
diff --git a/flang/lib/Semantics/openmp-utils.h 
b/flang/include/flang/Semantics/openmp-utils.h
similarity index 100%
rename from flang/lib/Semantics/openmp-utils.h
rename to flang/include/flang/Semantics/openmp-utils.h
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp 
b/flang/lib/Lower/OpenMP/OpenMP.cpp
index d1efd8e8d2ca7..f7a7dd8fbe6a0 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -34,6 +34,7 @@
 #include "flang/Parser/openmp-utils.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/openmp-directive-sets.h"
+#include "flang/Semantics/openmp-utils.h"
 #include "flang/Semantics/tools.h"
 #include "flang/Support/Flags.h"
 #include "flang/Support/OpenMP-utils.h"
@@ -3797,18 +3798,29 @@ static void genOMP(lower::AbstractConverter &converter, 
lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPCriticalConstruct &criticalConstruct) {
-  const auto &cd = std::get(criticalConstruct.t);
-  List clauses =
-  makeClauses(std::get(cd.t), semaCtx);
+  const parser::OmpDirectiveSpecification &beginSpec =
+  criticalConstruct.BeginDir();
+  List clauses = makeClauses(beginSpec.Clauses(), semaCtx);
 
   ConstructQueue queue{buildConstructQueue(
-  converter.getFirOpBuilder().getModule(), semaCtx, eval, cd.source,
+  converter.getFirOpBuilder().getModule(), semaCtx, eval, beginSpec.source,
   llvm::omp::Directive::OMPD_critical, clauses)};
 
-  const auto &name = std::get>(cd.t);
+  std::optional critName;
+  const parser::OmpArgumentList &args = beginSpec.Arguments();
+  if (!args.v.empty()) {
+// All of these things should be guaranteed to exist after semantic checks.
+auto *object = parser::Unwrap(args.v.front());
+assert(object && "Expecting object as argument");
+auto *designator = semantics::omp::GetDesignatorFromObj(*object);
+assert(designator && "Expecting desginator in argument");
+auto *name = semantics::getDesignatorNameIfDataRef(*designator);
+assert(name && "Expecting dataref in designator");
+critName = *name;
+  }
   mlir::Location currentLocation = converter.getCurrentLocation();
   genCriticalOp(converter, symTable, semaCtx, eval, currentLocation, queue,
-queue.begin(), name);
+queue.begin(), critName);
 }
 
 static void genOMP(lower::AbstractConverter &converter, lower::SymMap 
&symTable,
diff --git a/flang/lib/Parser/openmp-parsers.cpp 
b/flang/lib/Parser/openmp-parsers.cpp
index 84d1e81bfd9be..ab23e7d70de4f 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1758,17 +1758,8 @@ 
TYPE_PARSER(sourced(construct(
 TYPE_PARSER(construct(Parser{}) ||
 construct(Parser{}))
 
-// 2.13.2 OMP CRITICAL
-TYPE_PARSER(sta

[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-04 Thread via llvm-branch-commits

llvmbot wrote:




@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)


Changes

This allows not having the END CRITICAL directive in certain situations. Update 
semantic checks and symbol resolution.

---

Patch is 24.11 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/152007.diff


16 Files Affected:

- (modified) flang/include/flang/Parser/parse-tree.h (+3-3) 
- (renamed) flang/include/flang/Semantics/openmp-utils.h () 
- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+18-6) 
- (modified) flang/lib/Parser/openmp-parsers.cpp (+2-11) 
- (modified) flang/lib/Parser/unparse.cpp (+1-3) 
- (modified) flang/lib/Semantics/check-omp-atomic.cpp (+1-1) 
- (modified) flang/lib/Semantics/check-omp-loop.cpp (+1-1) 
- (modified) flang/lib/Semantics/check-omp-metadirective.cpp (+1-2) 
- (modified) flang/lib/Semantics/check-omp-structure.cpp (+81-43) 
- (modified) flang/lib/Semantics/openmp-utils.cpp (+1-1) 
- (modified) flang/lib/Semantics/resolve-directives.cpp (+3-3) 
- (modified) flang/lib/Semantics/resolve-names.cpp (+33-36) 
- (modified) flang/lib/Semantics/unparse-with-symbols.cpp (-14) 
- (modified) flang/test/Parser/OpenMP/critical-unparse-with-symbols.f90 (+2-2) 
- (modified) flang/test/Semantics/OpenMP/sync-critical01.f90 (+4-4) 
- (modified) flang/test/Semantics/OpenMP/sync-critical02.f90 (+4-4) 


``diff
diff --git a/flang/include/flang/Parser/parse-tree.h 
b/flang/include/flang/Parser/parse-tree.h
index 8302e40984af0..e72190f019dd1 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4986,9 +4986,9 @@ struct OmpEndCriticalDirective {
   CharBlock source;
   std::tuple> t;
 };
-struct OpenMPCriticalConstruct {
-  TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct);
-  std::tuple t;
+
+struct OpenMPCriticalConstruct : public OmpBlockConstruct {
+  INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct, 
OmpBlockConstruct);
 };
 
 // 2.11.3 allocate -> ALLOCATE [(variable-name-list)] [clause]
diff --git a/flang/lib/Semantics/openmp-utils.h 
b/flang/include/flang/Semantics/openmp-utils.h
similarity index 100%
rename from flang/lib/Semantics/openmp-utils.h
rename to flang/include/flang/Semantics/openmp-utils.h
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp 
b/flang/lib/Lower/OpenMP/OpenMP.cpp
index d1efd8e8d2ca7..f7a7dd8fbe6a0 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -34,6 +34,7 @@
 #include "flang/Parser/openmp-utils.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/openmp-directive-sets.h"
+#include "flang/Semantics/openmp-utils.h"
 #include "flang/Semantics/tools.h"
 #include "flang/Support/Flags.h"
 #include "flang/Support/OpenMP-utils.h"
@@ -3797,18 +3798,29 @@ static void genOMP(lower::AbstractConverter &converter, 
lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPCriticalConstruct &criticalConstruct) {
-  const auto &cd = std::get(criticalConstruct.t);
-  List clauses =
-  makeClauses(std::get(cd.t), semaCtx);
+  const parser::OmpDirectiveSpecification &beginSpec =
+  criticalConstruct.BeginDir();
+  List clauses = makeClauses(beginSpec.Clauses(), semaCtx);
 
   ConstructQueue queue{buildConstructQueue(
-  converter.getFirOpBuilder().getModule(), semaCtx, eval, cd.source,
+  converter.getFirOpBuilder().getModule(), semaCtx, eval, beginSpec.source,
   llvm::omp::Directive::OMPD_critical, clauses)};
 
-  const auto &name = std::get>(cd.t);
+  std::optional critName;
+  const parser::OmpArgumentList &args = beginSpec.Arguments();
+  if (!args.v.empty()) {
+// All of these things should be guaranteed to exist after semantic checks.
+auto *object = parser::Unwrap(args.v.front());
+assert(object && "Expecting object as argument");
+auto *designator = semantics::omp::GetDesignatorFromObj(*object);
+assert(designator && "Expecting desginator in argument");
+auto *name = semantics::getDesignatorNameIfDataRef(*designator);
+assert(name && "Expecting dataref in designator");
+critName = *name;
+  }
   mlir::Location currentLocation = converter.getCurrentLocation();
   genCriticalOp(converter, symTable, semaCtx, eval, currentLocation, queue,
-queue.begin(), name);
+queue.begin(), critName);
 }
 
 static void genOMP(lower::AbstractConverter &converter, lower::SymMap 
&symTable,
diff --git a/flang/lib/Parser/openmp-parsers.cpp 
b/flang/lib/Parser/openmp-parsers.cpp
index 84d1e81bfd9be..ab23e7d70de4f 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1758,17 +1758,8 @@ 
TYPE_PARSER(sourced(construct(
 TYPE_PARSER(construct(Parser{}) ||
 construct(Parser{}))
 
-// 2.13.2 OMP CRITICAL
-TYPE_PARSER(startOmpLine >>
-sourced(construct(
-verbatim("END CRITICAL"_tok), maybe(p

[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-04 Thread via llvm-branch-commits

llvmbot wrote:



@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-parser

Author: Krzysztof Parzyszek (kparzysz)


Changes

This allows not having the END CRITICAL directive in certain situations. Update 
semantic checks and symbol resolution.

---

Patch is 24.11 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/152007.diff


16 Files Affected:

- (modified) flang/include/flang/Parser/parse-tree.h (+3-3) 
- (renamed) flang/include/flang/Semantics/openmp-utils.h () 
- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+18-6) 
- (modified) flang/lib/Parser/openmp-parsers.cpp (+2-11) 
- (modified) flang/lib/Parser/unparse.cpp (+1-3) 
- (modified) flang/lib/Semantics/check-omp-atomic.cpp (+1-1) 
- (modified) flang/lib/Semantics/check-omp-loop.cpp (+1-1) 
- (modified) flang/lib/Semantics/check-omp-metadirective.cpp (+1-2) 
- (modified) flang/lib/Semantics/check-omp-structure.cpp (+81-43) 
- (modified) flang/lib/Semantics/openmp-utils.cpp (+1-1) 
- (modified) flang/lib/Semantics/resolve-directives.cpp (+3-3) 
- (modified) flang/lib/Semantics/resolve-names.cpp (+33-36) 
- (modified) flang/lib/Semantics/unparse-with-symbols.cpp (-14) 
- (modified) flang/test/Parser/OpenMP/critical-unparse-with-symbols.f90 (+2-2) 
- (modified) flang/test/Semantics/OpenMP/sync-critical01.f90 (+4-4) 
- (modified) flang/test/Semantics/OpenMP/sync-critical02.f90 (+4-4) 


``diff
diff --git a/flang/include/flang/Parser/parse-tree.h 
b/flang/include/flang/Parser/parse-tree.h
index 8302e40984af0..e72190f019dd1 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4986,9 +4986,9 @@ struct OmpEndCriticalDirective {
   CharBlock source;
   std::tuple> t;
 };
-struct OpenMPCriticalConstruct {
-  TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct);
-  std::tuple t;
+
+struct OpenMPCriticalConstruct : public OmpBlockConstruct {
+  INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct, 
OmpBlockConstruct);
 };
 
 // 2.11.3 allocate -> ALLOCATE [(variable-name-list)] [clause]
diff --git a/flang/lib/Semantics/openmp-utils.h 
b/flang/include/flang/Semantics/openmp-utils.h
similarity index 100%
rename from flang/lib/Semantics/openmp-utils.h
rename to flang/include/flang/Semantics/openmp-utils.h
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp 
b/flang/lib/Lower/OpenMP/OpenMP.cpp
index d1efd8e8d2ca7..f7a7dd8fbe6a0 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -34,6 +34,7 @@
 #include "flang/Parser/openmp-utils.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/openmp-directive-sets.h"
+#include "flang/Semantics/openmp-utils.h"
 #include "flang/Semantics/tools.h"
 #include "flang/Support/Flags.h"
 #include "flang/Support/OpenMP-utils.h"
@@ -3797,18 +3798,29 @@ static void genOMP(lower::AbstractConverter &converter, 
lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPCriticalConstruct &criticalConstruct) {
-  const auto &cd = std::get(criticalConstruct.t);
-  List clauses =
-  makeClauses(std::get(cd.t), semaCtx);
+  const parser::OmpDirectiveSpecification &beginSpec =
+  criticalConstruct.BeginDir();
+  List clauses = makeClauses(beginSpec.Clauses(), semaCtx);
 
   ConstructQueue queue{buildConstructQueue(
-  converter.getFirOpBuilder().getModule(), semaCtx, eval, cd.source,
+  converter.getFirOpBuilder().getModule(), semaCtx, eval, beginSpec.source,
   llvm::omp::Directive::OMPD_critical, clauses)};
 
-  const auto &name = std::get>(cd.t);
+  std::optional critName;
+  const parser::OmpArgumentList &args = beginSpec.Arguments();
+  if (!args.v.empty()) {
+// All of these things should be guaranteed to exist after semantic checks.
+auto *object = parser::Unwrap(args.v.front());
+assert(object && "Expecting object as argument");
+auto *designator = semantics::omp::GetDesignatorFromObj(*object);
+assert(designator && "Expecting desginator in argument");
+auto *name = semantics::getDesignatorNameIfDataRef(*designator);
+assert(name && "Expecting dataref in designator");
+critName = *name;
+  }
   mlir::Location currentLocation = converter.getCurrentLocation();
   genCriticalOp(converter, symTable, semaCtx, eval, currentLocation, queue,
-queue.begin(), name);
+queue.begin(), critName);
 }
 
 static void genOMP(lower::AbstractConverter &converter, lower::SymMap 
&symTable,
diff --git a/flang/lib/Parser/openmp-parsers.cpp 
b/flang/lib/Parser/openmp-parsers.cpp
index 84d1e81bfd9be..ab23e7d70de4f 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1758,17 +1758,8 @@ 
TYPE_PARSER(sourced(construct(
 TYPE_PARSER(construct(Parser{}) ||
 construct(Parser{}))
 
-// 2.13.2 OMP CRITICAL
-TYPE_PARSER(startOmpLine >>
-sourced(construct(
-

[llvm-branch-commits] [flang] [flang][OpenMP] Make OpenMPCriticalConstruct follow block structure (PR #152007)

2025-08-04 Thread Krzysztof Parzyszek via llvm-branch-commits

https://github.com/kparzysz created 
https://github.com/llvm/llvm-project/pull/152007

This allows not having the END CRITICAL directive in certain situations. Update 
semantic checks and symbol resolution.

>From d57337ec742dd2ace259ab7599b587265723840d Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek 
Date: Mon, 4 Aug 2025 07:59:47 -0500
Subject: [PATCH] [flang][OpenMP] Make OpenMPCriticalConstruct follow block
 structure

This allows not having the END CRITICAL directive in certain situations.
Update semantic checks and symbol resolution.
---
 flang/include/flang/Parser/parse-tree.h   |   6 +-
 .../flang}/Semantics/openmp-utils.h   |   0
 flang/lib/Lower/OpenMP/OpenMP.cpp |  24 +++-
 flang/lib/Parser/openmp-parsers.cpp   |  13 +-
 flang/lib/Parser/unparse.cpp  |   4 +-
 flang/lib/Semantics/check-omp-atomic.cpp  |   2 +-
 flang/lib/Semantics/check-omp-loop.cpp|   2 +-
 .../lib/Semantics/check-omp-metadirective.cpp |   3 +-
 flang/lib/Semantics/check-omp-structure.cpp   | 124 --
 flang/lib/Semantics/openmp-utils.cpp  |   2 +-
 flang/lib/Semantics/resolve-directives.cpp|   6 +-
 flang/lib/Semantics/resolve-names.cpp |  69 +-
 flang/lib/Semantics/unparse-with-symbols.cpp  |  14 --
 .../OpenMP/critical-unparse-with-symbols.f90  |   4 +-
 .../test/Semantics/OpenMP/sync-critical01.f90 |   8 +-
 .../test/Semantics/OpenMP/sync-critical02.f90 |   8 +-
 16 files changed, 155 insertions(+), 134 deletions(-)
 rename flang/{lib => include/flang}/Semantics/openmp-utils.h (100%)

diff --git a/flang/include/flang/Parser/parse-tree.h 
b/flang/include/flang/Parser/parse-tree.h
index 8302e40984af0..e72190f019dd1 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4986,9 +4986,9 @@ struct OmpEndCriticalDirective {
   CharBlock source;
   std::tuple> t;
 };
-struct OpenMPCriticalConstruct {
-  TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct);
-  std::tuple t;
+
+struct OpenMPCriticalConstruct : public OmpBlockConstruct {
+  INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPCriticalConstruct, 
OmpBlockConstruct);
 };
 
 // 2.11.3 allocate -> ALLOCATE [(variable-name-list)] [clause]
diff --git a/flang/lib/Semantics/openmp-utils.h 
b/flang/include/flang/Semantics/openmp-utils.h
similarity index 100%
rename from flang/lib/Semantics/openmp-utils.h
rename to flang/include/flang/Semantics/openmp-utils.h
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp 
b/flang/lib/Lower/OpenMP/OpenMP.cpp
index d1efd8e8d2ca7..f7a7dd8fbe6a0 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -34,6 +34,7 @@
 #include "flang/Parser/openmp-utils.h"
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/openmp-directive-sets.h"
+#include "flang/Semantics/openmp-utils.h"
 #include "flang/Semantics/tools.h"
 #include "flang/Support/Flags.h"
 #include "flang/Support/OpenMP-utils.h"
@@ -3797,18 +3798,29 @@ static void genOMP(lower::AbstractConverter &converter, 
lower::SymMap &symTable,
semantics::SemanticsContext &semaCtx,
lower::pft::Evaluation &eval,
const parser::OpenMPCriticalConstruct &criticalConstruct) {
-  const auto &cd = std::get(criticalConstruct.t);
-  List clauses =
-  makeClauses(std::get(cd.t), semaCtx);
+  const parser::OmpDirectiveSpecification &beginSpec =
+  criticalConstruct.BeginDir();
+  List clauses = makeClauses(beginSpec.Clauses(), semaCtx);
 
   ConstructQueue queue{buildConstructQueue(
-  converter.getFirOpBuilder().getModule(), semaCtx, eval, cd.source,
+  converter.getFirOpBuilder().getModule(), semaCtx, eval, beginSpec.source,
   llvm::omp::Directive::OMPD_critical, clauses)};
 
-  const auto &name = std::get>(cd.t);
+  std::optional critName;
+  const parser::OmpArgumentList &args = beginSpec.Arguments();
+  if (!args.v.empty()) {
+// All of these things should be guaranteed to exist after semantic checks.
+auto *object = parser::Unwrap(args.v.front());
+assert(object && "Expecting object as argument");
+auto *designator = semantics::omp::GetDesignatorFromObj(*object);
+assert(designator && "Expecting desginator in argument");
+auto *name = semantics::getDesignatorNameIfDataRef(*designator);
+assert(name && "Expecting dataref in designator");
+critName = *name;
+  }
   mlir::Location currentLocation = converter.getCurrentLocation();
   genCriticalOp(converter, symTable, semaCtx, eval, currentLocation, queue,
-queue.begin(), name);
+queue.begin(), critName);
 }
 
 static void genOMP(lower::AbstractConverter &converter, lower::SymMap 
&symTable,
diff --git a/flang/lib/Parser/openmp-parsers.cpp 
b/flang/lib/Parser/openmp-parsers.cpp
index 84d1e81bfd9be..ab23e7d70de4f 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1758,17 +1758,8 @@ 
TYPE_PARSER(source