Skip to content

Conversation

@kparzysz
Copy link
Contributor

Block-associated constructs have, as their body, either a strictly- or a loosely-structured block. In the former case the end-directive is optional.

The existing parser required the end-directive to be present in all cases.

Note:
The definitions of these blocks in the OpenMP spec exclude cases where the block contains more than one construct, and the first one is BLOCK/ENDBLOCK. For example, the following is invalid:

  !$omp target
  block         ! This cannot be a strictly-structured block, but
    continue    ! a loosely-structured block cannot start with
  endblock      ! BLOCK/ENDBLOCK
  continue      !
  !$omp end target

Block-associated constructs have, as their body, either a strictly-
or a loosely-structured block. In the former case the end-directive
is optional.

The existing parser required the end-directive to be present in all
cases.

Note:
The definitions of these blocks in the OpenMP spec exclude cases
where the block contains more than one construct, and the first one
is BLOCK/ENDBLOCK. For example, the following is invalid:
```
  !$omp target
  block         ! This cannot be a strictly-structured block, but
    continue    ! a loosely-structured block cannot start with
  endblock      ! BLOCK/ENDBLOCK
  continue      !
  !$omp end target
```
@kparzysz kparzysz requested review from Stylie777 and tblah July 23, 2025 19:51
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:semantics flang:parser labels Jul 23, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2025

@llvm/pr-subscribers-flang-openmp
@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-parser

Author: Krzysztof Parzyszek (kparzysz)

Changes

Block-associated constructs have, as their body, either a strictly- or a loosely-structured block. In the former case the end-directive is optional.

The existing parser required the end-directive to be present in all cases.

Note:
The definitions of these blocks in the OpenMP spec exclude cases where the block contains more than one construct, and the first one is BLOCK/ENDBLOCK. For example, the following is invalid:

  !$omp target
  block         ! This cannot be a strictly-structured block, but
    continue    ! a loosely-structured block cannot start with
  endblock      ! BLOCK/ENDBLOCK
  continue      !
  !$omp end target

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

6 Files Affected:

  • (modified) flang/include/flang/Parser/parse-tree.h (+2-1)
  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+15-7)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+60-4)
  • (modified) flang/lib/Parser/unparse.cpp (+7-5)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+13-8)
  • (added) flang/test/Parser/OpenMP/block-construct.f90 (+165)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 0b3dec1010312..3a28f6f9731c3 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -5119,7 +5119,8 @@ struct OmpEndBlockDirective {
 
 struct OpenMPBlockConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenMPBlockConstruct);
-  std::tuple<OmpBeginBlockDirective, Block, OmpEndBlockDirective> t;
+  std::tuple<OmpBeginBlockDirective, Block, std::optional<OmpEndBlockDirective>>
+      t;
 };
 
 // OpenMP directives enclosing do loop
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 4c2d7badef382..12089d6caa5fe 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -411,8 +411,12 @@ static void processHostEvalClauses(lower::AbstractConverter &converter,
                   std::get<parser::OmpBeginBlockDirective>(ompConstruct.t);
               beginClauseList =
                   &std::get<parser::OmpClauseList>(beginDirective.t);
-              endClauseList = &std::get<parser::OmpClauseList>(
-                  std::get<parser::OmpEndBlockDirective>(ompConstruct.t).t);
+              if (auto &endDirective =
+                      std::get<std::optional<parser::OmpEndBlockDirective>>(
+                          ompConstruct.t)) {
+                endClauseList =
+                    &std::get<parser::OmpClauseList>(endDirective->t);
+              }
             },
             [&](const parser::OpenMPLoopConstruct &ompConstruct) {
               const auto &beginDirective =
@@ -422,9 +426,10 @@ static void processHostEvalClauses(lower::AbstractConverter &converter,
 
               if (auto &endDirective =
                       std::get<std::optional<parser::OmpEndLoopDirective>>(
-                          ompConstruct.t))
+                          ompConstruct.t)) {
                 endClauseList =
                     &std::get<parser::OmpClauseList>(endDirective->t);
+              }
             },
             [&](const auto &) {}},
         ompEval->u);
@@ -3713,16 +3718,19 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
                    const parser::OpenMPBlockConstruct &blockConstruct) {
   const auto &beginBlockDirective =
       std::get<parser::OmpBeginBlockDirective>(blockConstruct.t);
-  const auto &endBlockDirective =
-      std::get<parser::OmpEndBlockDirective>(blockConstruct.t);
   mlir::Location currentLocation =
       converter.genLocation(beginBlockDirective.source);
   const auto origDirective =
       std::get<parser::OmpBlockDirective>(beginBlockDirective.t).v;
   List<Clause> clauses = makeClauses(
       std::get<parser::OmpClauseList>(beginBlockDirective.t), semaCtx);
-  clauses.append(makeClauses(
-      std::get<parser::OmpClauseList>(endBlockDirective.t), semaCtx));
+
+  if (const auto &endBlockDirective =
+          std::get<std::optional<parser::OmpEndBlockDirective>>(
+              blockConstruct.t)) {
+    clauses.append(makeClauses(
+        std::get<parser::OmpClauseList>(endBlockDirective->t), semaCtx));
+  }
 
   assert(llvm::omp::blockConstructSet.test(origDirective) &&
          "Expected block construct");
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index d349d8ceb0bb5..1f04392639cf9 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1208,6 +1208,54 @@ TYPE_PARSER(sourced(
         maybe(Parser<OmpClauseList>{}),
         pure(OmpDirectiveSpecification::Flags::None))))
 
+static bool IsFortranBlockConstruct(const ExecutionPartConstruct &epc) {
+  // ExecutionPartConstruct -> ExecutableConstruct
+  //   -> Indirection<BlockConstruct>
+  if (auto *ec{std::get_if<ExecutableConstruct>(&epc.u)}) {
+    return std::holds_alternative<common::Indirection<BlockConstruct>>(ec->u);
+  } else {
+    return false;
+  }
+}
+
+struct StrictlyStructuredBlockParser {
+  using resultType = Block;
+
+  std::optional<resultType> Parse(ParseState &state) const {
+    if (auto epc{Parser<ExecutionPartConstruct>{}.Parse(state)}) {
+      if (IsFortranBlockConstruct(*epc)) {
+        Block block;
+        block.emplace_back(std::move(*epc));
+        return std::move(block);
+      }
+    }
+    return std::nullopt;
+  }
+};
+
+struct LooselyStructuredBlockParser {
+  using resultType = Block;
+
+  std::optional<resultType> Parse(ParseState &state) const {
+    Block body;
+    if (auto epc{attempt(Parser<ExecutionPartConstruct>{}).Parse(state)}) {
+      if (!IsFortranBlockConstruct(*epc)) {
+        body.emplace_back(std::move(*epc));
+        if (auto &&blk{attempt(block).Parse(state)}) {
+          for (auto &&s : *blk) {
+            body.emplace_back(std::move(s));
+          }
+        }
+      } else {
+        // Fail if the first construct is BLOCK.
+        return std::nullopt;
+      }
+    }
+    // Empty body is ok.
+    return std::move(body);
+  }
+};
+
 TYPE_PARSER(sourced(construct<OmpNothingDirective>("NOTHING" >> ok)))
 
 TYPE_PARSER(sourced(construct<OpenMPUtilityConstruct>(
@@ -1570,12 +1618,17 @@ TYPE_PARSER(
             Parser<OpenMPInteropConstruct>{})) /
     endOfLine)
 
+static constexpr auto StandaloneDirectiveLookahead{//
+    "TARGET ENTER DATA"_sptok || "TARGET_ENTER_DATA"_sptok || //
+    "TARGET EXIT DATA"_sptok || "TARGET_EXIT"_sptok || //
+    "TARGET UPDATE"_sptok || "TARGET_UPDATE"_sptok};
+
 // Directives enclosing structured-block
 TYPE_PARSER(
     // In this context "TARGET UPDATE" can be parsed as a TARGET directive
     // followed by an UPDATE clause. This is the only combination at the
     // moment, exclude it explicitly.
-    (!("TARGET UPDATE"_sptok || "TARGET_UPDATE"_sptok)) >=
+    (!StandaloneDirectiveLookahead) >=
     construct<OmpBlockDirective>(first(
         "MASKED" >> pure(llvm::omp::Directive::OMPD_masked),
         "MASTER" >> pure(llvm::omp::Directive::OMPD_master),
@@ -1749,9 +1802,12 @@ TYPE_PARSER(sourced(
         block, maybe(Parser<OmpEndAssumeDirective>{} / endOmpLine))))
 
 // Block Construct
-TYPE_PARSER(construct<OpenMPBlockConstruct>(
-    Parser<OmpBeginBlockDirective>{} / endOmpLine, block,
-    Parser<OmpEndBlockDirective>{} / endOmpLine))
+TYPE_PARSER( //
+    construct<OpenMPBlockConstruct>(Parser<OmpBeginBlockDirective>{},
+        StrictlyStructuredBlockParser{},
+        maybe(Parser<OmpEndBlockDirective>{})) ||
+    construct<OpenMPBlockConstruct>(Parser<OmpBeginBlockDirective>{},
+        LooselyStructuredBlockParser{}, Parser<OmpEndBlockDirective>{}))
 
 // OMP SECTIONS Directive
 TYPE_PARSER(construct<OmpSectionsDirective>(first(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 8ed16905b5099..fc15d46a8c727 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2898,11 +2898,13 @@ class UnparseVisitor {
     Put("\n");
     EndOpenMP();
     Walk(std::get<Block>(x.t), "");
-    BeginOpenMP();
-    Word("!$OMP END ");
-    Walk(std::get<OmpEndBlockDirective>(x.t));
-    Put("\n");
-    EndOpenMP();
+    if (auto &&end{std::get<std::optional<OmpEndBlockDirective>>(x.t)}) {
+      BeginOpenMP();
+      Word("!$OMP END ");
+      Walk(*end);
+      Put("\n");
+      EndOpenMP();
+    }
   }
   void Unparse(const OpenMPLoopConstruct &x) {
     BeginOpenMP();
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 8264e1d5e8fd9..243d7ae722796 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -782,12 +782,15 @@ void OmpStructureChecker::CheckTargetNest(const parser::OpenMPConstruct &c) {
 
 void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
   const auto &beginBlockDir{std::get<parser::OmpBeginBlockDirective>(x.t)};
-  const auto &endBlockDir{std::get<parser::OmpEndBlockDirective>(x.t)};
+  const auto &endBlockDir{
+      std::get<std::optional<parser::OmpEndBlockDirective>>(x.t)};
   const auto &beginDir{std::get<parser::OmpBlockDirective>(beginBlockDir.t)};
-  const auto &endDir{std::get<parser::OmpBlockDirective>(endBlockDir.t)};
   const parser::Block &block{std::get<parser::Block>(x.t)};
 
-  CheckMatching<parser::OmpBlockDirective>(beginDir, endDir);
+  if (endBlockDir) {
+    const auto &endDir{std::get<parser::OmpBlockDirective>(endBlockDir->t)};
+    CheckMatching<parser::OmpBlockDirective>(beginDir, endDir);
+  }
 
   PushContextAndClauseSets(beginDir.source, beginDir.v);
   if (llvm::omp::allTargetSet.test(GetContext().directive)) {
@@ -837,14 +840,14 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
     bool foundNowait{false};
     parser::CharBlock NowaitSource;
 
-    auto catchCopyPrivateNowaitClauses = [&](const auto &dir, bool endDir) {
+    auto catchCopyPrivateNowaitClauses = [&](const auto &dir, bool isEnd) {
       for (auto &clause : std::get<parser::OmpClauseList>(dir.t).v) {
         if (clause.Id() == llvm::omp::Clause::OMPC_copyprivate) {
           for (const auto &ompObject : GetOmpObjectList(clause)->v) {
             const auto *name{parser::Unwrap<parser::Name>(ompObject)};
             if (Symbol * symbol{name->symbol}) {
               if (singleCopyprivateSyms.count(symbol)) {
-                if (endDir) {
+                if (isEnd) {
                   context_.Warn(common::UsageWarning::OpenMPUsage, name->source,
                       "The COPYPRIVATE clause with '%s' is already used on the SINGLE directive"_warn_en_US,
                       name->ToString());
@@ -858,7 +861,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
                     "'%s' appears in more than one COPYPRIVATE clause on the END SINGLE directive"_err_en_US,
                     name->ToString());
               } else {
-                if (endDir) {
+                if (isEnd) {
                   endSingleCopyprivateSyms.insert(symbol);
                 } else {
                   singleCopyprivateSyms.insert(symbol);
@@ -871,7 +874,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
             context_.Say(clause.source,
                 "At most one NOWAIT clause can appear on the SINGLE directive"_err_en_US);
           } else {
-            foundNowait = !endDir;
+            foundNowait = !isEnd;
           }
           if (!NowaitSource.ToString().size()) {
             NowaitSource = clause.source;
@@ -880,7 +883,9 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
       }
     };
     catchCopyPrivateNowaitClauses(beginBlockDir, false);
-    catchCopyPrivateNowaitClauses(endBlockDir, true);
+    if (endBlockDir) {
+      catchCopyPrivateNowaitClauses(*endBlockDir, true);
+    }
     unsigned version{context_.langOptions().OpenMPVersion};
     if (version <= 52 && NowaitSource.ToString().size() &&
         (singleCopyprivateSyms.size() || endSingleCopyprivateSyms.size())) {
diff --git a/flang/test/Parser/OpenMP/block-construct.f90 b/flang/test/Parser/OpenMP/block-construct.f90
new file mode 100644
index 0000000000000..83f0f7f276631
--- /dev/null
+++ b/flang/test/Parser/OpenMP/block-construct.f90
@@ -0,0 +1,165 @@
+!RUN: %flang_fc1 -fdebug-unparse -fopenmp %s | FileCheck --ignore-case --check-prefix="UNPARSE" %s
+!RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp %s | FileCheck --check-prefix="PARSE-TREE" %s
+
+subroutine f00(x, y)
+  implicit none
+  integer :: x, y
+  !$omp target map(x, y)
+  x = y + 1
+  y = 2 * x
+  !$omp end target
+end
+
+!UNPARSE: SUBROUTINE f00 (x, y)
+!UNPARSE:  IMPLICIT NONE
+!UNPARSE:  INTEGER x, y
+!UNPARSE: !$OMP TARGET  MAP(x,y)
+!UNPARSE:   x=y+1_4
+!UNPARSE:   y=2_4*x
+!UNPARSE: !$OMP END TARGET
+!UNPARSE: END SUBROUTINE
+
+!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPBlockConstruct
+!PARSE-TREE: | OmpBeginBlockDirective
+!PARSE-TREE: | | OmpBlockDirective -> llvm::omp::Directive = target
+!PARSE-TREE: | | OmpClauseList -> OmpClause -> Map -> OmpMapClause
+!PARSE-TREE: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | | | OmpObject -> Designator -> DataRef -> Name = 'y'
+!PARSE-TREE: | | | bool = 'true'
+!PARSE-TREE: | Block
+!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'x=y+1_4'
+!PARSE-TREE: | | | Variable = 'x'
+!PARSE-TREE: | | | | Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | | | Expr = 'y+1_4'
+!PARSE-TREE: | | | | Add
+!PARSE-TREE: | | | | | Expr = 'y'
+!PARSE-TREE: | | | | | | Designator -> DataRef -> Name = 'y'
+!PARSE-TREE: | | | | | Expr = '1_4'
+!PARSE-TREE: | | | | | | LiteralConstant -> IntLiteralConstant = '1'
+!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'y=2_4*x'
+!PARSE-TREE: | | | Variable = 'y'
+!PARSE-TREE: | | | | Designator -> DataRef -> Name = 'y'
+!PARSE-TREE: | | | Expr = '2_4*x'
+!PARSE-TREE: | | | | Multiply
+!PARSE-TREE: | | | | | Expr = '2_4'
+!PARSE-TREE: | | | | | | LiteralConstant -> IntLiteralConstant = '2'
+!PARSE-TREE: | | | | | Expr = 'x'
+!PARSE-TREE: | | | | | | Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | OmpEndBlockDirective
+!PARSE-TREE: | | OmpBlockDirective -> llvm::omp::Directive = target
+!PARSE-TREE: | | OmpClauseList ->
+
+
+subroutine f01(x, y)
+  implicit none
+  integer :: x, y
+  !$omp target map(x, y)
+  block
+    x = y + 1
+    y = 2 * x
+  endblock
+  ! No end-directive
+end
+
+!UNPARSE: SUBROUTINE f01 (x, y)
+!UNPARSE:  IMPLICIT NONE
+!UNPARSE:  INTEGER x, y
+!UNPARSE: !$OMP TARGET  MAP(x,y)
+!UNPARSE:  BLOCK
+!UNPARSE:    x=y+1_4
+!UNPARSE:    y=2_4*x
+!UNPARSE:  END BLOCK
+!UNPARSE: END SUBROUTINE
+
+!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPBlockConstruct
+!PARSE-TREE: | OmpBeginBlockDirective
+!PARSE-TREE: | | OmpBlockDirective -> llvm::omp::Directive = target
+!PARSE-TREE: | | OmpClauseList -> OmpClause -> Map -> OmpMapClause
+!PARSE-TREE: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | | | OmpObject -> Designator -> DataRef -> Name = 'y'
+!PARSE-TREE: | | | bool = 'true'
+!PARSE-TREE: | Block
+!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> BlockConstruct
+!PARSE-TREE: | | | BlockStmt ->
+!PARSE-TREE: | | | BlockSpecificationPart -> SpecificationPart
+!PARSE-TREE: | | | | ImplicitPart ->
+!PARSE-TREE: | | | Block
+!PARSE-TREE: | | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'x=y+1_4'
+!PARSE-TREE: | | | | | Variable = 'x'
+!PARSE-TREE: | | | | | | Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | | | | | Expr = 'y+1_4'
+!PARSE-TREE: | | | | | | Add
+!PARSE-TREE: | | | | | | | Expr = 'y'
+!PARSE-TREE: | | | | | | | | Designator -> DataRef -> Name = 'y'
+!PARSE-TREE: | | | | | | | Expr = '1_4'
+!PARSE-TREE: | | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
+!PARSE-TREE: | | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'y=2_4*x'
+!PARSE-TREE: | | | | | Variable = 'y'
+!PARSE-TREE: | | | | | | Designator -> DataRef -> Name = 'y'
+!PARSE-TREE: | | | | | Expr = '2_4*x'
+!PARSE-TREE: | | | | | | Multiply
+!PARSE-TREE: | | | | | | | Expr = '2_4'
+!PARSE-TREE: | | | | | | | | LiteralConstant -> IntLiteralConstant = '2'
+!PARSE-TREE: | | | | | | | Expr = 'x'
+!PARSE-TREE: | | | | | | | | Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | | | EndBlockStmt ->
+
+
+subroutine f02(x, y)
+  implicit none
+  integer :: x, y
+  !$omp target map(x, y)
+  block
+    x = y + 1
+    y = 2 * x
+  endblock
+  ! End-directive present
+  !$omp end target
+end
+
+!UNPARSE: SUBROUTINE f02 (x, y)
+!UNPARSE:  IMPLICIT NONE
+!UNPARSE:  INTEGER x, y
+!UNPARSE: !$OMP TARGET  MAP(x,y)
+!UNPARSE:  BLOCK
+!UNPARSE:    x=y+1_4
+!UNPARSE:    y=2_4*x
+!UNPARSE:  END BLOCK
+!UNPARSE: !$OMP END TARGET
+!UNPARSE: END SUBROUTINE
+
+!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPBlockConstruct
+!PARSE-TREE: | OmpBeginBlockDirective
+!PARSE-TREE: | | OmpBlockDirective -> llvm::omp::Directive = target
+!PARSE-TREE: | | OmpClauseList -> OmpClause -> Map -> OmpMapClause
+!PARSE-TREE: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | | | OmpObject -> Designator -> DataRef -> Name = 'y'
+!PARSE-TREE: | | | bool = 'true'
+!PARSE-TREE: | Block
+!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> BlockConstruct
+!PARSE-TREE: | | | BlockStmt ->
+!PARSE-TREE: | | | BlockSpecificationPart -> SpecificationPart
+!PARSE-TREE: | | | | ImplicitPart ->
+!PARSE-TREE: | | | Block
+!PARSE-TREE: | | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'x=y+1_4'
+!PARSE-TREE: | | | | | Variable = 'x'
+!PARSE-TREE: | | | | | | Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | | | | | Expr = 'y+1_4'
+!PARSE-TREE: | | | | | | Add
+!PARSE-TREE: | | | | | | | Expr = 'y'
+!PARSE-TREE: | | | | | | | | Designator -> DataRef -> Name = 'y'
+!PARSE-TREE: | | | | | | | Expr = '1_4'
+!PARSE-TREE: | | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
+!PARSE-TREE: | | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'y=2_4*x'
+!PARSE-TREE: | | | | | Variable = 'y'
+!PARSE-TREE: | | | | | | Designator -> DataRef -> Name = 'y'
+!PARSE-TREE: | | | | | Expr = '2_4*x'
+!PARSE-TREE: | | | | | | Multiply
+!PARSE-TREE: | | | | | | | Expr = '2_4'
+!PARSE-TREE: | | | | | | | | LiteralConstant -> IntLiteralConstant = '2'
+!PARSE-TREE: | | | | | | | Expr = 'x'
+!PARSE-TREE: | | | | | | | | Designator -> DataRef -> Name = 'x'
+!PARSE-TREE: | | | EndBlockStmt ->
+!PARSE-TREE: | OmpEndBlockDirective
+!PARSE-TREE: | | OmpBlockDirective -> llvm::omp::Directive = target
+!PARSE-TREE: | | OmpClauseList ->

Comment on lines 1628 to 1630
// In this context "TARGET UPDATE" can be parsed as a TARGET directive
// followed by an UPDATE clause. This is the only combination at the
// moment, exclude it explicitly.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this comment needs updating. Maybe this also needs a lit test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if (auto epc{attempt(Parser<ExecutionPartConstruct>{}).Parse(state)}) {
if (!IsFortranBlockConstruct(*epc)) {
body.emplace_back(std::move(*epc));
if (auto &&blk{attempt(block).Parse(state)}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry if this is a silly question - where is block defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not silly, lol. It's defined here: https://github.com/llvm/llvm-project/blob/main/flang/lib/Parser/misc-parsers.h#L43

There is a bunch of such pre-created parsers in various files that you can stumble upon when looking at the rest of the parser code.

@github-actions
Copy link

github-actions bot commented Jul 24, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@tblah tblah 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 for the explanation

@kparzysz kparzysz merged commit 1ba3859 into llvm:main Jul 24, 2025
9 checks passed
@kparzysz kparzysz deleted the users/kparzysz/structured-block branch July 24, 2025 13:59
@luporl
Copy link
Contributor

luporl commented Jul 25, 2025

This caused a regression in Fujitsu testsuite, test 0981_0034.f90. Flang doesn't finish the compilation, even after more than 4 hours. With this PR reverted (and the commit that fixes a warning) the compilation finishes in a few seconds.
Below is a stacktrace of the process after ~1 minute running:

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/leandro.lupori/git/flang/install/bin/flang -fc1 -triple aarch64-unknown-linux-gnu -emit-llvm-bc -I /home/leandro.lupori/git/llvm-test-suite/Fujitsu/Fortran/0981 -D NDEBUG -ffixed-line-length=72 -flto=full -mrelocation-model pic -pic-level 2 -pic-is-pie -ffast-math -target-cpu generic -target-feature +outline-atomics -target-feature +v8.4a -target-feature +ccidx -target-feature +complxnum -target-feature +crc -target-feature +dit -target-feature +dotprod -target-feature +flagm -target-feature +fp-armv8 -target-feature +fp16fml -target-feature +fullfp16 -target-feature +jsconv -target-feature +lse -target-feature +neon -target-feature +pauth -target-feature +ras -target-feature +rcpc -target-feature +rdm -target-feature +sve -mvscale-max=2 -mvscale-min=2 -floop-interchange -vectorize-loops -vectorize-slp -fversion-loops-for-stride -module-dir Fujitsu/Fortran/0981/Fujitsu-Fortran-0981_0034.d -fopenmp -resource-dir /home/leandro.lupori/git/flang/install/lib/clang/22 -mframe-pointer=non-leaf -mllvm -treat-scalable-fixed-error-as-warning=false -O3 -o 0981_0034.f90.o Fortran/0981/0981_0034.f90
  #0 0x0000aaaade6fec24 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/leandro.lupori/git/flang/install/bin/flang+0x3b66c24)
  #1 0x0000aaaade6fc5b4 llvm::sys::RunSignalHandlers() (/home/leandro.lupori/git/flang/install/bin/flang+0x3b645b4)
  #2 0x0000aaaade6ffa34 SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
  #3 0x0000ffff8bf2d598 (linux-vdso.so.1+0x598)
  #4 0x0000aaaadeee8868 Fortran::common::CountedReference<Fortran::parser::Message>::Drop() Clauses.cpp:0:0
  #5 0x0000aaaae01d093c void Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::AlternativesParser<Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Absent, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpAbsentClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Acquire>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::AcqRel>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::AdjustArgs, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpAdjustArgsClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Affinity, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpAffinityClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Align, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpAlignClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Aligned, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpAlignedClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Allocate, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpAllocateClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::AppendArgs, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpAppendArgsClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Allocator, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::ApplyConstructor<Fortran::parser::Scalar<Fortran::parser::Integer<Fortran::common::Indirection<Fortran::parser::Expr, false>>>, Fortran::parser::ApplyConstructor<Fortran::parser::Integer<Fortran::common::Indirection<Fortran::parser::Expr, false>>, Fortran::parser::ApplyConstructor<Fortran::common::Indirection<Fortran::parser::Expr, false>, Fortran::parser::Parser<Fortran::parser::Expr>>>>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::At, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpAtClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::AtomicDefaultMemOrder, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpAtomicDefaultMemOrderClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Bind, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpBindClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Capture>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Collapse, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::ApplyConstructor<Fortran::parser::Scalar<Fortran::parser::Integer<Fortran::parser::Constant<Fortran::common::Indirection<Fortran::parser::Expr, false>>>>, Fortran::parser::ApplyConstructor<Fortran::parser::Integer<Fortran::parser::Constant<Fortran::common::Indirection<Fortran::parser::Expr, false>>>, Fortran::parser::ApplyConstructor<Fortran::parser::Constant<Fortran::common::Indirection<Fortran::parser::Expr, false>>, Fortran::parser::ApplyConstructor<Fortran::common::Indirection<Fortran::parser::Expr, false>, Fortran::parser::Parser<Fortran::parser::Expr>>>>>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Compare>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Contains, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpContainsClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Copyin, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpObjectList>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Copyprivate, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpObjectList>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, true>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Default, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpDefaultClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Defaultmap, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpDefaultmapClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Depend, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpDependClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Destroy, Fortran::parser::MaybeParser<Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::ApplyConstructor<Fortran::parser::OmpDestroyClause, Fortran::parser::Parser<Fortran::parser::OmpObject>>, Fortran::parser::TokenStringMatch<false, false>>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Device, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpDeviceClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::DeviceType, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpDeviceTypeClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::DistSchedule, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::MaybeParser<Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::Scalar<Fortran::parser::Integer<Fortran::common::Indirection<Fortran::parser::Expr, false>>>, Fortran::parser::ApplyConstructor<Fortran::parser::Integer<Fortran::common::Indirection<Fortran::parser::Expr, false>>, Fortran::parser::ApplyConstructor<Fortran::common::Indirection<Fortran::parser::Expr, false>, Fortran::parser::Parser<Fortran::parser::Expr>>>>>>>, Fortran::parser::TokenStringMatch<false, false>>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpDoacrossClause>, Fortran::parser::TokenStringMatch<false, false>>>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::DynamicAllocators>>>>, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause, Fortran::parser::ApplyConstructor<Fortran::parser::OmpClause::Enter, Fortran::parser::SequenceParser<Fortran::parser::TokenStringMatch<false, false>, Fortran::parser::FollowParser<Fortran::parser::Parser<Fortran::parser::OmpObjectList>, Fortran::parser::TokenStringMatch<false, false>>>>>>>::ParseRest<1>(std::optional<Fortran::parser::OmpClause>&, Fortran::parser::ParseState&, Fortran::parser::ParseState&) const openmp-parsers.cpp:0:0
...
#3077 0x0000aaaac7a70760 in Fortran::parser::Parsing::Parse(llvm::raw_ostream&) ()
#3078 0x0000aaaac61fb8dc in Fortran::frontend::FrontendAction::runParse(bool) ()
#3079 0x0000aaaac6201204 in Fortran::frontend::CodeGenAction::beginSourceFileAction() ()
#3080 0x0000aaaac61fb0a4 in Fortran::frontend::FrontendAction::beginSourceFile(Fortran::frontend::CompilerInstance&, Fortran::frontend::FrontendInputFile const&) ()
#3081 0x0000aaaac61e313c in Fortran::frontend::CompilerInstance::executeAction(Fortran::frontend::FrontendAction&) ()
#3082 0x0000aaaac61ff68c in Fortran::frontend::executeCompilerInvocation(Fortran::frontend::CompilerInstance*) ()
#3083 0x0000aaaac5d16684 in fc1_main(llvm::ArrayRef<char const*>, char const*) ()
#3084 0x0000aaaac5d15780 in main ()

Frames 0-5 are from killing the process with SIGABRT, but it stops on frame 255. Frames 3077-3084 are from attaching gdb in another run.

@kparzysz
Copy link
Contributor Author

Fix: #150617

mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
…50298)

Block-associated constructs have, as their body, either a strictly- or a
loosely-structured block. In the former case the end-directive is
optional.

The existing parser required the end-directive to be present in all
cases.

Note:
The definitions of these blocks in the OpenMP spec exclude cases where
the block contains more than one construct, and the first one is
BLOCK/ENDBLOCK. For example, the following is invalid:
```
  !$omp target
  block         ! This cannot be a strictly-structured block, but
    continue    ! a loosely-structured block cannot start with
  endblock      ! BLOCK/ENDBLOCK
  continue      !
  !$omp end target
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants