[flang-commits] [flang] [flang][openacc] Make OpenACC block construct parse errors less verbose. (PR #131042)

Andre Kuhlenschmidt via flang-commits flang-commits at lists.llvm.org
Mon Mar 17 13:53:15 PDT 2025


https://github.com/akuhlens updated https://github.com/llvm/llvm-project/pull/131042

>From d48f6ad3ecd00b84e80959c26def48da7827200f Mon Sep 17 00:00:00 2001
From: Andre Kuhlenschmidt <akuhlenschmi at nvidia.com>
Date: Thu, 6 Mar 2025 17:14:20 -0800
Subject: [PATCH 1/6] ACC token and context duplication fixed

---
 flang/lib/Parser/message.cpp             |  40 +++-
 flang/lib/Parser/openacc-parsers.cpp     |   9 +-
 flang/test/Driver/debug-parsing-log.f90  |  18 +-
 flang/test/Parser/acc-data-statement.f90 | 245 +++++++++++++++++++++++
 flang/test/Parser/acc.f                  |  96 +++++++++
 5 files changed, 392 insertions(+), 16 deletions(-)
 create mode 100644 flang/test/Parser/acc-data-statement.f90
 create mode 100644 flang/test/Parser/acc.f

diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp
index 69e4814bf246c..0b8d300189a1b 100644
--- a/flang/lib/Parser/message.cpp
+++ b/flang/lib/Parser/message.cpp
@@ -272,6 +272,10 @@ static llvm::raw_ostream::Colors PrefixColor(Severity severity) {
   return llvm::raw_ostream::SAVEDCOLOR;
 }
 
+// FIXME: Make these configurable, based on verbosity level.
+const int MAX_CONTEXTS_EMITTED = 2;
+const bool OMIT_SHARED_CONTEXTS = true;
+
 void Message::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked,
     bool echoSourceLine) const {
   std::optional<ProvenanceRange> provenanceRange{GetProvenanceRange(allCooked)};
@@ -279,12 +283,38 @@ void Message::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked,
   sources.EmitMessage(o, provenanceRange, ToString(), Prefix(severity()),
       PrefixColor(severity()), echoSourceLine);
   bool isContext{attachmentIsContext_};
+  int contextsEmitted{isContext ? 1 : 0};
+  // Emit attachments.
   for (const Message *attachment{attachment_.get()}; attachment;
-       attachment = attachment->attachment_.get()) {
+      attachment = attachment->attachment_.get()) {
     Severity severity = isContext ? Severity::Context : attachment->severity();
-    sources.EmitMessage(o, attachment->GetProvenanceRange(allCooked),
-        attachment->ToString(), Prefix(severity), PrefixColor(severity),
-        echoSourceLine);
+    auto emitAttachment = [&]() {
+      sources.EmitMessage(o, attachment->GetProvenanceRange(allCooked),
+          attachment->ToString(), Prefix(severity), PrefixColor(severity),
+          echoSourceLine);
+    };
+    // TODO isContext is not used correctly here.
+    if (attachment->attachmentIsContext_) {
+      // Truncate the number of contexts emitted.
+      if (contextsEmitted <= MAX_CONTEXTS_EMITTED) {
+        emitAttachment();
+        contextsEmitted += 1;
+      }
+      if (OMIT_SHARED_CONTEXTS) {
+        // Skip less specific contexts at the same location.
+        for (const Message *next_attachment{attachment->attachment_.get()};
+            next_attachment && next_attachment->attachmentIsContext_ &&
+            next_attachment->AtSameLocation(*attachment);
+            next_attachment = next_attachment->attachment_.get()) {
+          attachment = next_attachment;
+        }
+        // NB, this loop increments `attachment` one more time after the
+        // previous loop is done advancing it to the last context at the same
+        // location.
+      }
+    } else {
+      emitAttachment();
+    }
   }
 }
 
@@ -298,7 +328,7 @@ bool Message::operator==(const Message &that) const {
   }
   const Message *thatAttachment{that.attachment_.get()};
   for (const Message *attachment{attachment_.get()}; attachment;
-       attachment = attachment->attachment_.get()) {
+      attachment = attachment->attachment_.get()) {
     if (!thatAttachment || !attachment->AtSameLocation(*thatAttachment) ||
         attachment->ToString() != thatAttachment->ToString() ||
         attachment->severity() != thatAttachment->severity()) {
diff --git a/flang/lib/Parser/openacc-parsers.cpp b/flang/lib/Parser/openacc-parsers.cpp
index c78676664e0a3..6739bcc035fc2 100644
--- a/flang/lib/Parser/openacc-parsers.cpp
+++ b/flang/lib/Parser/openacc-parsers.cpp
@@ -19,8 +19,12 @@
 // OpenACC Directives and Clauses
 namespace Fortran::parser {
 
+// Only need to handle ! line comments because prescanning normalizes the
+// other types of line comments from fixed form.
 constexpr auto startAccLine{skipStuffBeforeStatement >>
-    ("!$ACC "_sptok || "C$ACC "_sptok || "*$ACC "_sptok)};
+    withMessage(
+        "expected OpenACC comment '!$ACC' (free-form), 'C$ACC', or '*$ACC' (fixed-form)"_err_en_US,
+        "!$ACC "_sptok)};
 constexpr auto endAccLine{space >> endOfLine};
 
 // Autogenerated clauses parser. Information is taken from ACC.td and the
@@ -225,7 +229,8 @@ TYPE_PARSER(startAccLine >> sourced(construct<AccEndBlockDirective>("END"_tok >>
 
 TYPE_PARSER(construct<OpenACCBlockConstruct>(
     Parser<AccBeginBlockDirective>{} / endAccLine, block,
-    Parser<AccEndBlockDirective>{} / endAccLine))
+    withMessage("expected OpenACC end block directive"_err_en_US,
+        Parser<AccEndBlockDirective>{} / endAccLine)))
 
 // Standalone constructs
 TYPE_PARSER(construct<OpenACCStandaloneConstruct>(
diff --git a/flang/test/Driver/debug-parsing-log.f90 b/flang/test/Driver/debug-parsing-log.f90
index 7297163109450..4e56add386ef8 100644
--- a/flang/test/Driver/debug-parsing-log.f90
+++ b/flang/test/Driver/debug-parsing-log.f90
@@ -12,14 +12,14 @@
 ! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: in the context: IMPLICIT statement
 ! CHECK-NEXT:   END PROGRAM
 ! CHECK-NEXT:   ^
-! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: in the context: implicit part
-! CHECK-NEXT:   END PROGRAM
-! CHECK-NEXT:   ^
-! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: in the context: specification part
-! CHECK-NEXT:   END PROGRAM
-! CHECK-NEXT:   ^
-! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: in the context: main program
-! CHECK-NEXT:   END PROGRAM
-! CHECK-NEXT:   ^
+
+
+
+
+
+
+
+
+
 
 END PROGRAM
diff --git a/flang/test/Parser/acc-data-statement.f90 b/flang/test/Parser/acc-data-statement.f90
new file mode 100644
index 0000000000000..1e369a0db3780
--- /dev/null
+++ b/flang/test/Parser/acc-data-statement.f90
@@ -0,0 +1,245 @@
+! RUN: not %flang_fc1 -fsyntax-only -fopenacc %s 2>&1 | FileCheck %s
+program acc_data_test
+    implicit none
+    integer :: a(100), b(100), c(100), d(100)
+    integer :: i, s ! FIXME: if s is named sum you get semantic errors.
+
+    ! Positive tests
+
+    ! Basic data construct in program body
+    !$acc data copy(a, b) create(c)
+    a = 1
+    b = 2
+    c = a + b
+    !$acc end data
+    print *, "After first data region"
+
+    ! Data construct within IF block
+    if (.true.) then
+        !$acc data copyout(a)
+        a = a + 1
+        !$acc end data
+        print *, "Inside if block"
+    end if
+
+    ! Data construct within DO loop
+    do i = 1, 10
+        !$acc data present(a)
+        a(i) = a(i) * 2
+        !$acc end data
+        print *, "Loop iteration", i
+    end do
+
+    ! Nested data constructs
+    !$acc data copyin(a)
+    s = 0
+    !$acc data copy(s)
+    s = s + 1
+    !$acc end data
+    print *, "After nested data"
+    !$acc end data
+
+    ! Negative tests  
+    ! Basic data construct in program body
+    !$acc data copy(a, b) create(d)
+    a = 1
+    b = 2
+    d = a + b
+!   !$acc end data
+    print *, "After first data region"
+
+    ! Data construct within IF block
+    if (.true.) then
+        !$acc data copyout(a)
+        a = a + 1
+!       !$acc end data
+        print *, "Inside if block"
+        ! First error in the file.
+        !CHECK: acc-data-statement.f90:
+        !CHECK-SAME: [[ELINE1:[0-9]+]]:{{[0-9]+}}:
+        !CHECK-SAME: error: expected OpenACC end block directive
+        !CHECK-NEXT: end if
+        !CHECK-NEXT: ^ 
+        !CHECK-NEXT: in the context: OpenACC construct
+        !CHECK-NEXT: !$acc data copyout(a)
+        !CHECK-NEXT: ^
+        !CHECK-NEXT: in the context: IF construct
+        !CHECK-NEXT: if (.true.) then
+        !CHECK-NEXT: ^
+        !CHECK-NEXT: error: expected OpenACC end block directive
+        !CHECK-NEXT: end if
+        !CHECK-NEXT: ^ 
+        !CHECK-NEXT: in the context: OpenACC construct
+        !CHECK-NEXT: !$acc data copyout(a)
+        !CHECK-NEXT: ^
+        !CHECK-NEXT: in the context: IF construct
+        !CHECK-NEXT: if (.true.) then
+        !CHECK-NEXT: ^
+    end if
+
+    ! Data construct within DO loop
+    do i = 1, 10
+        !$acc data present(a)
+        a(i) = a(i) * 2
+!       !$acc end data
+        print *, "Loop iteration", i
+        !CHECK: acc-data-statement.f90:
+        !CHECK-NOT:  [[ELINE1]]
+        !CHECK-SAME: [[ELINE2:[0-9]+]]:{{[0-9]+}}:
+        !CHECK-SAME: error: expected OpenACC end block directive
+        !CHECK-NEXT: end do
+        !CHECK-NEXT: ^ 
+        !CHECK-NEXT: in the context: OpenACC construct
+        !CHECK-NEXT: !$acc data present(a)
+        !CHECK-NEXT: ^
+        !CHECK-NEXT: in the context: DO construct
+        !CHECK-NEXT: do i = 1, 10
+        !CHECK-NEXT: ^
+        !CHECK-NEXT: error: expected OpenACC end block directive
+        !CHECK-NEXT: end do 
+        !CHECK-NEXT: ^ 
+        !CHECK-NEXT: in the context: OpenACC construct
+        !CHECK-NEXT: !$acc data present(a)
+        !CHECK-NEXT: ^
+        !CHECK-NEXT: in the context: DO construct
+        !CHECK-NEXT: do i = 1, 10
+        !CHECK-NEXT: ^
+    end do
+
+    ! Nested data constructs
+    !$acc data copyin(a)
+    s = 0
+    !$acc data copy(s)
+    s = s + 1
+!   !$acc end data
+    print *, "After nested data"
+!   !$acc end data
+
+    print *, "Program finished"
+    !CHECK: acc-data-statement.f90:
+    !CHECK-NOT:  [[ELINE2]]
+    !CHECK-SAME: [[ELINE3:[0-9]+]]:{{[0-9]+}}:
+    !CHECK-SAME: error: expected OpenACC end block directive
+    !CHECK-NEXT: contains
+    !CHECK-NEXT: ^ 
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copy(s)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: execution part
+    !CHECK-NEXT: !$acc data copy(a, b) create(c)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: error: expected OpenACC end block directive
+    !CHECK-NEXT: contains
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copy(s)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copyin(a)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: error: expected OpenACC end block directive
+    !CHECK-NEXT: contains
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copyin(a)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: execution part
+    !CHECK-NEXT: !$acc data copy(a, b) create(c)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: error: expected OpenACC end block directive
+    !CHECK-NEXT: contains
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copy(s)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copy(a, b) create(d)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: error: expected OpenACC end block directive
+    !CHECK-NEXT: contains
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copy(s)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copyin(a)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: error: expected OpenACC end block directive
+    !CHECK-NEXT: contains
+    !CHECK-NEXT: ^ 
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copyin(a)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copy(a, b) create(d)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: error: expected OpenACC end block directive
+    !CHECK-NEXT: contains
+    !CHECK-NEXT: ^ 
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copy(a, b) create(d)
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: execution part
+    !CHECK-NEXT: !$acc data copy(a, b) create(c)
+    !CHECK-NEXT: ^
+contains
+    subroutine positive_process_array(x)
+        integer, intent(inout) :: x(:)
+        
+        ! Data construct in subroutine
+        !$acc data copy(x)
+        x = x + 1
+        !$acc end data
+        print *, "Subroutine finished"
+    end subroutine
+
+    function positive_compute_sum(x) result(total)
+        integer, intent(in) :: x(:)
+        integer :: total
+        
+        ! Data construct in function
+        !$acc data copyin(x) copy(total)
+        total = sum(x)
+        !$acc end data
+        print *, "Function finished"
+    end function
+    
+    subroutine negative_process_array(x)
+        integer, intent(inout) :: x(:)
+        
+        ! Data construct in subroutine
+        !$acc data copy(x)
+        x = x + 1
+!       !$acc end data
+        print *, "Subroutine finished"
+        !CHECK: error: expected OpenACC directive
+        !CHECK-NEXT: !$acc data copy(x)
+        !CHECK-NEXT: ^ 
+        !CHECK-NEXT: in the context: specification construct
+        !CHECK-NEXT: !$acc data copy(x)
+        !CHECK-NEXT: ^
+        !CHECK-NEXT: in the context: specification part
+        !CHECK-NEXT: integer, intent(inout) :: x(:)
+        !CHECK-NEXT: ^
+    end subroutine
+
+    function negative_compute_sum(x) result(total)
+        integer, intent(in) :: x(:)
+        integer :: total
+        total = sum(x)
+        ! Data construct in function
+        !$acc data copyin(x) copy(total)
+        total = total + x
+!       !$acc end data
+        print *, "Function finished"
+        !CHECK: error: expected OpenACC end block directive
+        !CHECK-NEXT: end function
+        !CHECK-NEXT: ^ 
+        !CHECK-NEXT: in the context: OpenACC construct
+        !CHECK-NEXT: !$acc data copyin(x) copy(total)
+        !CHECK-NEXT: ^
+        !CHECK-NEXT: in the context: execution part
+        !CHECK-NEXT: total = sum(x)
+        !CHECK-NEXT: ^
+    end function
+end program acc_data_test
\ No newline at end of file
diff --git a/flang/test/Parser/acc.f b/flang/test/Parser/acc.f
new file mode 100644
index 0000000000000..b0c3927772568
--- /dev/null
+++ b/flang/test/Parser/acc.f
@@ -0,0 +1,96 @@
+! RUN: %flang_fc1 -fsyntax-only -fopenacc %s 2>&1
+C Test file for OpenACC directives in fixed-form Fortran
+      PROGRAM ACCTEST
+      IMPLICIT NONE
+      INTEGER :: N, I, J
+      PARAMETER (N=100)
+      REAL :: A(N), B(N), C(N), D(N)
+      REAL :: SUM
+
+C Initialize arrays
+      DO I = 1, N
+         A(I) = I * 1.0
+         B(I) = I * 2.0
+         C(I) = 0.0
+         D(I) = 1.0
+      END DO
+
+C Basic data construct using C$ACC
+C$ACC DATA COPYIN(A,B) COPYOUT(C)
+      DO I = 1, N
+         C(I) = A(I) + B(I)
+      END DO
+C$ACC END DATA
+
+* Parallel construct with loop using *$ACC
+*$ACC PARALLEL PRESENT(A,B,C)
+*$ACC LOOP
+      DO I = 1, N
+         C(I) = C(I) * 2.0
+      END DO
+*$ACC END PARALLEL
+
+C Nested loops with collapse - C$ACC style
+C$ACC PARALLEL LOOP COLLAPSE(2)
+      DO I = 1, N
+         DO J = 1, N
+            A(J) = A(J) + B(J)
+         END DO
+      END DO
+C$ACC END PARALLEL LOOP
+
+* Combined parallel loop with reduction - *$ACC style
+      SUM = 0.0
+*$ACC PARALLEL LOOP REDUCTION(+:SUM)
+      DO I = 1, N
+         SUM = SUM + C(I)
+      END DO
+*$ACC END PARALLEL LOOP
+
+C Kernels construct - C$ACC with continuation
+C$ACC KERNELS 
+C$ACC+ COPYOUT(A)
+      DO I = 1, N
+         A(I) = A(I) * 2.0
+      END DO
+C$ACC END KERNELS
+
+* Data construct with update - *$ACC with continuation
+*$ACC DATA COPY(B)
+*$ACC+ PRESENT(D)
+      B(1) = 999.0
+*$ACC UPDATE HOST(B(1:1))
+      PRINT *, 'B(1) = ', B(1)
+*$ACC END DATA
+
+C Mixed style directives in nested constructs
+C$ACC DATA COPY(A,B,C)
+*$ACC PARALLEL LOOP
+      DO I = 1, N
+         A(I) = B(I) + C(I)
+      END DO
+*$ACC END PARALLEL LOOP
+C$ACC END DATA
+
+* Subroutine call within data region - *$ACC style
+*$ACC DATA COPY(A,B,C)
+      CALL SUB1(A, B, C, N)
+*$ACC END DATA
+
+      PRINT *, 'Sum = ', SUM
+      END PROGRAM
+
+C Subroutine with mixed ACC directive styles
+      SUBROUTINE SUB1(X, Y, Z, M)
+      INTEGER M, I
+      REAL X(M), Y(M), Z(M)
+
+*$ACC PARALLEL PRESENT(X,Y)
+C$ACC LOOP PRIVATE(I)
+      DO I = 1, M
+         Z(I) = X(I) + Y(I)
+      END DO
+C$ACC END LOOP
+*$ACC END PARALLEL
+      RETURN
+      END SUBROUTINE 
\ No newline at end of file

>From 158383d2fe50c597985adf9811f899d31081c901 Mon Sep 17 00:00:00 2001
From: Andre Kuhlenschmidt <akuhlenschmi at nvidia.com>
Date: Wed, 12 Mar 2025 17:07:20 -0700
Subject: [PATCH 2/6] add more descriptive error message and remove newlines

---
 flang/lib/Parser/openacc-parsers.cpp    |  2 +-
 flang/lib/Parser/stmt-parser.h          |  2 +-
 flang/test/Driver/debug-parsing-log.f90 | 16 +++-------------
 3 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/flang/lib/Parser/openacc-parsers.cpp b/flang/lib/Parser/openacc-parsers.cpp
index 6739bcc035fc2..41aedd47d58bb 100644
--- a/flang/lib/Parser/openacc-parsers.cpp
+++ b/flang/lib/Parser/openacc-parsers.cpp
@@ -23,7 +23,7 @@ namespace Fortran::parser {
 // other types of line comments from fixed form.
 constexpr auto startAccLine{skipStuffBeforeStatement >>
     withMessage(
-        "expected OpenACC comment '!$ACC' (free-form), 'C$ACC', or '*$ACC' (fixed-form)"_err_en_US,
+        "expected OpenACC directive sentinal: '!$ACC' (free-form), 'C$ACC', or '*$ACC' (fixed-form)"_err_en_US,
         "!$ACC "_sptok)};
 constexpr auto endAccLine{space >> endOfLine};
 
diff --git a/flang/lib/Parser/stmt-parser.h b/flang/lib/Parser/stmt-parser.h
index ee45c6fd5d38c..61e6b95f0ddbb 100644
--- a/flang/lib/Parser/stmt-parser.h
+++ b/flang/lib/Parser/stmt-parser.h
@@ -98,7 +98,7 @@ constexpr auto progUnitEndStmt{
                  "PROCEDURE"_tok || "MODULE"_tok || "SUBMODULE"_tok ||
                  "PROGRAM"_tok || "BLOCK DATA"_tok)};
 constexpr auto constructEndStmtErrorRecovery{
-    !progUnitEndStmt >> ("END"_tok >> SkipTo<'\n'>{} || ok)};
+    !progUnitEndStmt >> (("!$ACC "_sptok >> "END"_tok) || "END"_tok >> SkipTo<'\n'>{} || ok)};
 constexpr auto namedConstructEndStmtErrorRecovery{
     constructEndStmtErrorRecovery >> missingOptionalName};
 
diff --git a/flang/test/Driver/debug-parsing-log.f90 b/flang/test/Driver/debug-parsing-log.f90
index 4e56add386ef8..fdf52071ab956 100644
--- a/flang/test/Driver/debug-parsing-log.f90
+++ b/flang/test/Driver/debug-parsing-log.f90
@@ -2,24 +2,14 @@
 
 ! Below are just few lines extracted from the dump. The actual output is much _much_ bigger.
 
-! CHECK: {{.*[/\\]}}debug-parsing-log.f90:25:1: IMPLICIT statement
+! CHECK: {{.*[/\\]}}debug-parsing-log.f90:15:1: IMPLICIT statement
 ! CHECK-NEXT:  END PROGRAM
 ! CHECK-NEXT:  ^
 ! CHECK-NEXT:  fail 3
-! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: error: expected 'IMPLICIT NONE'
+! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:15:1: error: expected 'IMPLICIT NONE'
 ! CHECK-NEXT:   END PROGRAM
 ! CHECK-NEXT:   ^
-! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:25:1: in the context: IMPLICIT statement
+! CHECK-NEXT: {{.*[/\\]}}debug-parsing-log.f90:15:1: in the context: IMPLICIT statement
 ! CHECK-NEXT:   END PROGRAM
 ! CHECK-NEXT:   ^
-
-
-
-
-
-
-
-
-
-
 END PROGRAM

>From d6b4310ebcc408b933a8c4c47f47b9eaf6210155 Mon Sep 17 00:00:00 2001
From: Andre Kuhlenschmidt <akuhlenschmi at nvidia.com>
Date: Wed, 12 Mar 2025 17:11:18 -0700
Subject: [PATCH 3/6] back out mistake in constructEndStmtRecovery

---
 flang/lib/Parser/stmt-parser.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/lib/Parser/stmt-parser.h b/flang/lib/Parser/stmt-parser.h
index 61e6b95f0ddbb..ee45c6fd5d38c 100644
--- a/flang/lib/Parser/stmt-parser.h
+++ b/flang/lib/Parser/stmt-parser.h
@@ -98,7 +98,7 @@ constexpr auto progUnitEndStmt{
                  "PROCEDURE"_tok || "MODULE"_tok || "SUBMODULE"_tok ||
                  "PROGRAM"_tok || "BLOCK DATA"_tok)};
 constexpr auto constructEndStmtErrorRecovery{
-    !progUnitEndStmt >> (("!$ACC "_sptok >> "END"_tok) || "END"_tok >> SkipTo<'\n'>{} || ok)};
+    !progUnitEndStmt >> ("END"_tok >> SkipTo<'\n'>{} || ok)};
 constexpr auto namedConstructEndStmtErrorRecovery{
     constructEndStmtErrorRecovery >> missingOptionalName};
 

>From f6d59f6803c67627a651c2996392b286127862bb Mon Sep 17 00:00:00 2001
From: Andre Kuhlenschmidt <akuhlenschmi at nvidia.com>
Date: Fri, 14 Mar 2025 16:03:22 -0700
Subject: [PATCH 4/6] add recovery to openacc block parser

---
 flang/lib/Parser/openacc-parsers.cpp     | 25 +++++--
 flang/test/Parser/acc-data-statement.f90 | 83 ++++--------------------
 2 files changed, 33 insertions(+), 75 deletions(-)

diff --git a/flang/lib/Parser/openacc-parsers.cpp b/flang/lib/Parser/openacc-parsers.cpp
index 41aedd47d58bb..0144336da6d2d 100644
--- a/flang/lib/Parser/openacc-parsers.cpp
+++ b/flang/lib/Parser/openacc-parsers.cpp
@@ -23,9 +23,12 @@ namespace Fortran::parser {
 // other types of line comments from fixed form.
 constexpr auto startAccLine{skipStuffBeforeStatement >>
     withMessage(
-        "expected OpenACC directive sentinal: '!$ACC' (free-form), 'C$ACC', or '*$ACC' (fixed-form)"_err_en_US,
+        "expected OpenACC directive sentinal: !$ACC (free-form) / C$ACC or *$ACC (fixed-form)"_err_en_US,
         "!$ACC "_sptok)};
-constexpr auto endAccLine{space >> endOfLine};
+constexpr auto endAccLine{space >>
+    recovery(
+        withMessage("expected end of OpenACC directive"_err_en_US, endOfLine),
+        SkipTo<'\n'>{} || ok)};
 
 // Autogenerated clauses parser. Information is taken from ACC.td and the
 // parser is generated by tablegen.
@@ -225,12 +228,19 @@ TYPE_PARSER(sourced(construct<AccBeginBlockDirective>(
     sourced(Parser<AccBlockDirective>{}), Parser<AccClauseList>{})))
 
 TYPE_PARSER(startAccLine >> sourced(construct<AccEndBlockDirective>("END"_tok >>
-                                sourced(Parser<AccBlockDirective>{}))))
+                                sourced(recovery(Parser<AccBlockDirective>{},
+                                    construct<AccBlockDirective>(pure(
+                                        llvm::acc::Directive::ACCD_data)))))))
 
 TYPE_PARSER(construct<OpenACCBlockConstruct>(
     Parser<AccBeginBlockDirective>{} / endAccLine, block,
-    withMessage("expected OpenACC end block directive"_err_en_US,
-        Parser<AccEndBlockDirective>{} / endAccLine)))
+    // TODO: This still allows mismatched directives.
+    recovery(withMessage("expected OpenACC end block directive"_err_en_US,
+                 Parser<AccEndBlockDirective>{} / endAccLine),
+        // TODO: Is there a simpler way to build this?
+        sourced(construct<AccEndBlockDirective>(
+            sourced(construct<AccBlockDirective>(
+                pure(llvm::acc::Directive::ACCD_data))))))))
 
 // Standalone constructs
 TYPE_PARSER(construct<OpenACCStandaloneConstruct>(
@@ -254,8 +264,11 @@ TYPE_PARSER(sourced(construct<OpenACCEndConstruct>(
 TYPE_CONTEXT_PARSER("OpenACC construct"_en_US,
     startAccLine >>
         withMessage("expected OpenACC directive"_err_en_US,
-            first(construct<OpenACCConstruct>(Parser<OpenACCBlockConstruct>{}),
+            // Combined constructs before block constructs so we try to match
+            // the longest possible match first.
+            first(
                 construct<OpenACCConstruct>(Parser<OpenACCCombinedConstruct>{}),
+                construct<OpenACCConstruct>(Parser<OpenACCBlockConstruct>{}),
                 construct<OpenACCConstruct>(Parser<OpenACCLoopConstruct>{}),
                 construct<OpenACCConstruct>(
                     Parser<OpenACCStandaloneConstruct>{}),
diff --git a/flang/test/Parser/acc-data-statement.f90 b/flang/test/Parser/acc-data-statement.f90
index 1e369a0db3780..163f0601f9047 100644
--- a/flang/test/Parser/acc-data-statement.f90
+++ b/flang/test/Parser/acc-data-statement.f90
@@ -56,7 +56,6 @@ program acc_data_test
         print *, "Inside if block"
         ! First error in the file.
         !CHECK: acc-data-statement.f90:
-        !CHECK-SAME: [[ELINE1:[0-9]+]]:{{[0-9]+}}:
         !CHECK-SAME: error: expected OpenACC end block directive
         !CHECK-NEXT: end if
         !CHECK-NEXT: ^ 
@@ -66,15 +65,6 @@ program acc_data_test
         !CHECK-NEXT: in the context: IF construct
         !CHECK-NEXT: if (.true.) then
         !CHECK-NEXT: ^
-        !CHECK-NEXT: error: expected OpenACC end block directive
-        !CHECK-NEXT: end if
-        !CHECK-NEXT: ^ 
-        !CHECK-NEXT: in the context: OpenACC construct
-        !CHECK-NEXT: !$acc data copyout(a)
-        !CHECK-NEXT: ^
-        !CHECK-NEXT: in the context: IF construct
-        !CHECK-NEXT: if (.true.) then
-        !CHECK-NEXT: ^
     end if
 
     ! Data construct within DO loop
@@ -84,8 +74,6 @@ program acc_data_test
 !       !$acc end data
         print *, "Loop iteration", i
         !CHECK: acc-data-statement.f90:
-        !CHECK-NOT:  [[ELINE1]]
-        !CHECK-SAME: [[ELINE2:[0-9]+]]:{{[0-9]+}}:
         !CHECK-SAME: error: expected OpenACC end block directive
         !CHECK-NEXT: end do
         !CHECK-NEXT: ^ 
@@ -95,15 +83,6 @@ program acc_data_test
         !CHECK-NEXT: in the context: DO construct
         !CHECK-NEXT: do i = 1, 10
         !CHECK-NEXT: ^
-        !CHECK-NEXT: error: expected OpenACC end block directive
-        !CHECK-NEXT: end do 
-        !CHECK-NEXT: ^ 
-        !CHECK-NEXT: in the context: OpenACC construct
-        !CHECK-NEXT: !$acc data present(a)
-        !CHECK-NEXT: ^
-        !CHECK-NEXT: in the context: DO construct
-        !CHECK-NEXT: do i = 1, 10
-        !CHECK-NEXT: ^
     end do
 
     ! Nested data constructs
@@ -117,67 +96,31 @@ program acc_data_test
 
     print *, "Program finished"
     !CHECK: acc-data-statement.f90:
-    !CHECK-NOT:  [[ELINE2]]
-    !CHECK-SAME: [[ELINE3:[0-9]+]]:{{[0-9]+}}:
     !CHECK-SAME: error: expected OpenACC end block directive
     !CHECK-NEXT: contains
     !CHECK-NEXT: ^ 
     !CHECK-NEXT: in the context: OpenACC construct
     !CHECK-NEXT: !$acc data copy(s)
     !CHECK-NEXT: ^
-    !CHECK-NEXT: in the context: execution part
-    !CHECK-NEXT: !$acc data copy(a, b) create(c)
-    !CHECK-NEXT: ^
-    !CHECK-NEXT: error: expected OpenACC end block directive
-    !CHECK-NEXT: contains
-    !CHECK-NEXT: ^
-    !CHECK-NEXT: in the context: OpenACC construct
-    !CHECK-NEXT: !$acc data copy(s)
-    !CHECK-NEXT: ^
     !CHECK-NEXT: in the context: OpenACC construct
     !CHECK-NEXT: !$acc data copyin(a)
     !CHECK-NEXT: ^
-    !CHECK-NEXT: error: expected OpenACC end block directive
+    !CHECK: acc-data-statement.f90:
+    !CHECK-SAME: error: expected OpenACC end block directive
     !CHECK-NEXT: contains
     !CHECK-NEXT: ^
     !CHECK-NEXT: in the context: OpenACC construct
     !CHECK-NEXT: !$acc data copyin(a)
     !CHECK-NEXT: ^
-    !CHECK-NEXT: in the context: execution part
-    !CHECK-NEXT: !$acc data copy(a, b) create(c)
-    !CHECK-NEXT: ^
-    !CHECK-NEXT: error: expected OpenACC end block directive
-    !CHECK-NEXT: contains
-    !CHECK-NEXT: ^
-    !CHECK-NEXT: in the context: OpenACC construct
-    !CHECK-NEXT: !$acc data copy(s)
-    !CHECK-NEXT: ^
     !CHECK-NEXT: in the context: OpenACC construct
     !CHECK-NEXT: !$acc data copy(a, b) create(d)
     !CHECK-NEXT: ^
-    !CHECK-NEXT: error: expected OpenACC end block directive
-    !CHECK-NEXT: contains
-    !CHECK-NEXT: ^
-    !CHECK-NEXT: in the context: OpenACC construct
-    !CHECK-NEXT: !$acc data copy(s)
-    !CHECK-NEXT: ^
-    !CHECK-NEXT: in the context: OpenACC construct
-    !CHECK-NEXT: !$acc data copyin(a)
-    !CHECK-NEXT: ^
-    !CHECK-NEXT: error: expected OpenACC end block directive
+    !CHECK: acc-data-statement.f90:
+    !CHECK-SAME: error: expected OpenACC end block directive
     !CHECK-NEXT: contains
-    !CHECK-NEXT: ^ 
-    !CHECK-NEXT: in the context: OpenACC construct
-    !CHECK-NEXT: !$acc data copyin(a)
-    !CHECK-NEXT: ^
-    !CHECK-NEXT: in the context: OpenACC construct
-    !CHECK-NEXT: !$acc data copy(a, b) create(d)
     !CHECK-NEXT: ^
-    !CHECK-NEXT: error: expected OpenACC end block directive
-    !CHECK-NEXT: contains
-    !CHECK-NEXT: ^ 
     !CHECK-NEXT: in the context: OpenACC construct
-    !CHECK-NEXT: !$acc data copy(a, b) create(d)
+    !CHECK-NEXT: $acc data copy(a, b) create(d)
     !CHECK-NEXT: ^
     !CHECK-NEXT: in the context: execution part
     !CHECK-NEXT: !$acc data copy(a, b) create(c)
@@ -212,14 +155,15 @@ subroutine negative_process_array(x)
         x = x + 1
 !       !$acc end data
         print *, "Subroutine finished"
-        !CHECK: error: expected OpenACC directive
-        !CHECK-NEXT: !$acc data copy(x)
-        !CHECK-NEXT: ^ 
-        !CHECK-NEXT: in the context: specification construct
+        !CHECK: acc-data-statement.f90:
+        !CHECK-SAME: error: expected OpenACC end block directive
+        !CHECK-NEXT: end subroutine
+        !CHECK-NEXT: ^
+        !CHECK-NEXT: in the context: OpenACC construct
         !CHECK-NEXT: !$acc data copy(x)
         !CHECK-NEXT: ^
-        !CHECK-NEXT: in the context: specification part
-        !CHECK-NEXT: integer, intent(inout) :: x(:)
+        !CHECK-NEXT: in the context: SUBROUTINE subprogram
+        !CHECK-NEXT: subroutine negative_process_array(x)
         !CHECK-NEXT: ^
     end subroutine
 
@@ -232,7 +176,8 @@ function negative_compute_sum(x) result(total)
         total = total + x
 !       !$acc end data
         print *, "Function finished"
-        !CHECK: error: expected OpenACC end block directive
+        !CHECK: acc-data-statement.f90:
+        !CHECK-SAME: error: expected OpenACC end block directive
         !CHECK-NEXT: end function
         !CHECK-NEXT: ^ 
         !CHECK-NEXT: in the context: OpenACC construct

>From 4f8aeaf899ec05d380289e9b07b7b6b2fa153f0b Mon Sep 17 00:00:00 2001
From: Andre Kuhlenschmidt <akuhlenschmi at nvidia.com>
Date: Fri, 14 Mar 2025 17:09:31 -0700
Subject: [PATCH 5/6] clean-up

---
 flang/lib/Parser/message.cpp             | 12 ++++++----
 flang/test/Parser/acc-data-statement.f90 | 29 ++++++++++++++++--------
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp
index 0b8d300189a1b..862e61b29b57f 100644
--- a/flang/lib/Parser/message.cpp
+++ b/flang/lib/Parser/message.cpp
@@ -272,7 +272,7 @@ static llvm::raw_ostream::Colors PrefixColor(Severity severity) {
   return llvm::raw_ostream::SAVEDCOLOR;
 }
 
-// FIXME: Make these configurable, based on verbosity level.
+// TODO: Make these configurable, based on verbosity level.
 const int MAX_CONTEXTS_EMITTED = 2;
 const bool OMIT_SHARED_CONTEXTS = true;
 
@@ -282,10 +282,12 @@ void Message::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked,
   const AllSources &sources{allCooked.allSources()};
   sources.EmitMessage(o, provenanceRange, ToString(), Prefix(severity()),
       PrefixColor(severity()), echoSourceLine);
+  // Always refers to if the attachment in the loop below is a context.
   bool isContext{attachmentIsContext_};
-  int contextsEmitted{isContext ? 1 : 0};
+  int contextsEmitted{0};
   // Emit attachments.
   for (const Message *attachment{attachment_.get()}; attachment;
+      isContext = attachment->attachmentIsContext_,
       attachment = attachment->attachment_.get()) {
     Severity severity = isContext ? Severity::Context : attachment->severity();
     auto emitAttachment = [&]() {
@@ -293,10 +295,10 @@ void Message::Emit(llvm::raw_ostream &o, const AllCookedSources &allCooked,
           attachment->ToString(), Prefix(severity), PrefixColor(severity),
           echoSourceLine);
     };
-    // TODO isContext is not used correctly here.
-    if (attachment->attachmentIsContext_) {
+
+    if (isContext) {
       // Truncate the number of contexts emitted.
-      if (contextsEmitted <= MAX_CONTEXTS_EMITTED) {
+      if (contextsEmitted < MAX_CONTEXTS_EMITTED) {
         emitAttachment();
         contextsEmitted += 1;
       }
diff --git a/flang/test/Parser/acc-data-statement.f90 b/flang/test/Parser/acc-data-statement.f90
index 163f0601f9047..40c76b2561b24 100644
--- a/flang/test/Parser/acc-data-statement.f90
+++ b/flang/test/Parser/acc-data-statement.f90
@@ -41,7 +41,17 @@ program acc_data_test
 
     ! Negative tests  
     ! Basic data construct in program body
-    !$acc data copy(a, b) create(d)
+    !$acc data copy(a, b) create(d) bogus()
+    !CHECK: acc-data-statement.f90:
+    !CHECK-SAME: error: expected end of OpenACC directive
+    !CHECK-NEXT: !$acc data copy(a, b) create(d) bogus()
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: OpenACC construct
+    !CHECK-NEXT: !$acc data copy(a, b) create(d) bogus()
+    !CHECK-NEXT: ^
+    !CHECK-NEXT: in the context: execution part
+    !CHECK-NEXT: !$acc data copy(a, b) create(c)
+    !CHECK-NEXT: ^
     a = 1
     b = 2
     d = a + b
@@ -54,7 +64,6 @@ program acc_data_test
         a = a + 1
 !       !$acc end data
         print *, "Inside if block"
-        ! First error in the file.
         !CHECK: acc-data-statement.f90:
         !CHECK-SAME: error: expected OpenACC end block directive
         !CHECK-NEXT: end if
@@ -92,19 +101,19 @@ program acc_data_test
     s = s + 1
 !   !$acc end data
     print *, "After nested data"
-!   !$acc end data
-
-    print *, "Program finished"
+    !$acc end data  I forgot to comment this out.
     !CHECK: acc-data-statement.f90:
-    !CHECK-SAME: error: expected OpenACC end block directive
-    !CHECK-NEXT: contains
-    !CHECK-NEXT: ^ 
+    !CHECK-SAME: error: expected end of OpenACC directive
+    !CHECK-NEXT: !$acc end data  I forgot to comment this out.
+    !CHECK-NEXT: ^
     !CHECK-NEXT: in the context: OpenACC construct
     !CHECK-NEXT: !$acc data copy(s)
     !CHECK-NEXT: ^
     !CHECK-NEXT: in the context: OpenACC construct
     !CHECK-NEXT: !$acc data copyin(a)
     !CHECK-NEXT: ^
+    print *, "Program finished"
+
     !CHECK: acc-data-statement.f90:
     !CHECK-SAME: error: expected OpenACC end block directive
     !CHECK-NEXT: contains
@@ -113,14 +122,14 @@ program acc_data_test
     !CHECK-NEXT: !$acc data copyin(a)
     !CHECK-NEXT: ^
     !CHECK-NEXT: in the context: OpenACC construct
-    !CHECK-NEXT: !$acc data copy(a, b) create(d)
+    !CHECK-NEXT: !$acc data copy(a, b) create(d) bogus()
     !CHECK-NEXT: ^
     !CHECK: acc-data-statement.f90:
     !CHECK-SAME: error: expected OpenACC end block directive
     !CHECK-NEXT: contains
     !CHECK-NEXT: ^
     !CHECK-NEXT: in the context: OpenACC construct
-    !CHECK-NEXT: $acc data copy(a, b) create(d)
+    !CHECK-NEXT: $acc data copy(a, b) create(d) bogus()
     !CHECK-NEXT: ^
     !CHECK-NEXT: in the context: execution part
     !CHECK-NEXT: !$acc data copy(a, b) create(c)

>From e60c5711d72ffe6b1e9ff7179a6eabac29e3a922 Mon Sep 17 00:00:00 2001
From: Andre Kuhlenschmidt <akuhlenschmi at nvidia.com>
Date: Mon, 17 Mar 2025 13:52:58 -0700
Subject: [PATCH 6/6] fix off by one error in position

---
 flang/lib/Parser/openacc-parsers.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/lib/Parser/openacc-parsers.cpp b/flang/lib/Parser/openacc-parsers.cpp
index 0144336da6d2d..5816ad7bc37d8 100644
--- a/flang/lib/Parser/openacc-parsers.cpp
+++ b/flang/lib/Parser/openacc-parsers.cpp
@@ -236,7 +236,7 @@ TYPE_PARSER(construct<OpenACCBlockConstruct>(
     Parser<AccBeginBlockDirective>{} / endAccLine, block,
     // TODO: This still allows mismatched directives.
     recovery(withMessage("expected OpenACC end block directive"_err_en_US,
-                 Parser<AccEndBlockDirective>{} / endAccLine),
+                 attempt(Parser<AccEndBlockDirective>{} / endAccLine)),
         // TODO: Is there a simpler way to build this?
         sourced(construct<AccEndBlockDirective>(
             sourced(construct<AccBlockDirective>(



More information about the flang-commits mailing list